[edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes

Laszlo Ersek posted 1 patch 1 year, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
CryptoPkg/Readme.md | 48 ++++++++++----------
1 file changed, 24 insertions(+), 24 deletions(-)
[edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
Posted by Laszlo Ersek 1 year, 5 months ago
Commit 244ce33bdd2f ("CryptoPkg: Add Readme.md", 2022-10-24) had added the
long-awaited documentation on the dynamic crypto services. Fix some of the
typos and arguable grammar errors in "Readme.md". A few light
clarifications are also snuck in.

Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Best displayed as a word-diff for review, for example with the command
    
      git show --word-diff
    
    or on the web at
    
      https://pagure.io/lersek/edk2/c/a7269f170437?branch=cryptopkg_readme_typos

 CryptoPkg/Readme.md | 48 ++++++++++----------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/CryptoPkg/Readme.md b/CryptoPkg/Readme.md
index 946aa1e99e7d..9dd3a951836b 100644
--- a/CryptoPkg/Readme.md
+++ b/CryptoPkg/Readme.md
@@ -39,7 +39,7 @@ provides the smallest overall firmware overhead.
 
 ## Statically Linking Cryptographic Services
 
-The figure below shows an example of a firmware modules that requires the use of
+The figure below shows an example of a firmware module that requires the use of
 cryptographic services. The cryptographic services are provided by three library
 classes called BaseCryptLib, TlsLib, and HashApiLib. These library classes are
 implemented using APIs from the OpenSSL project that are abstracted by the
@@ -49,7 +49,7 @@ full C runtime library for firmware components. Instead, the CryptoPkg includes
 the smallest subset of services required to build the OpenSSL project in the
 private library class called IntrinsicLib.
 
-The CryptoPkg provides several instances if the BaseCryptLib and OpensslLib with
+The CryptoPkg provides several instances of the BaseCryptLib and OpensslLib with
 different cryptographic service features and performance optimizations. The
 platform developer must select the correct instances based on cryptographic
 service requirements in each UEFI/PI firmware phase (SEC, PEI, DXE, UEFI,
@@ -97,9 +97,9 @@ linking is not available for SEC or UEFI RT modules.
 
 The EDK II modules/libraries that require cryptographic services use the same
 BaseCryptLib/TlsLib/HashApiLib APIs. This means no source changes are required
-to use static linking or dynamic linking. It is a platform configuration options
-to select static linking or dynamic linking. This choice can be make globally,
-per firmware module type, or individual modules.
+to use static linking or dynamic linking. It is a platform configuration option
+to select static linking or dynamic linking. This choice can be made globally,
+per firmware module type, or for individual modules.
 
 ```
 +===================+    +===================+     +===================+
@@ -159,7 +159,7 @@ The table below provides a summary of the supported cryptographic services. It
 indicates if the family or service is deprecated or recommended to not be used.
 It also shows which *CryptLib library instances support the family or service.
 If a cell is blank then the service or family is always disabled and the
-`PcdCryptoServiceFamilyEnable` settings for that family or service is ignored.
+`PcdCryptoServiceFamilyEnable` setting for that family or service is ignored.
 If the cell is not blank, then the service or family is configurable using
 `PcdCryptoServiceFamilyEnable` as long as the correct OpensslLib or TlsLib is
 also configured.
@@ -234,10 +234,10 @@ phases (SEC, PEI, DXE, UEFI, SMM, UEFI RT).
 
 The following table can be used to help select the best OpensslLib instance for
 each phase. The Size column only shows the estimated size increase for a
-compressed IA32/X64 modules that uses the cryptographic services with
+compressed IA32/X64 module that uses the cryptographic services with
 `OpensslLib.inf` as the baseline size. The actual size increase depends on the
 specific set of enabled cryptographic services. If ECC services are not
-required, then size can be reduced by using OpensslLib.inf instead of
+required, then the size can be reduced by using OpensslLib.inf instead of
 `OpensslLibFull.inf`. Performance optimization requires a size increase.
 
 | OpensslLib Instance     | SSL | ECC | Perf Opt | CPU Arch | Size  |
@@ -371,10 +371,10 @@ settings.
 
 ### UEFI Runtime Driver Library Mappings
 
-UEFI Runtime Drivers only supports static linking of cryptographic services.
-The following library mappings are recommended for UEFI Runtime Drivers. It uses
-the runtime specific version of the BaseCryptLib and the null version of the
-TlsLib because TLS services are not typically used in runtime.
+UEFI Runtime Drivers only support static linking of cryptographic services.
+The following library mappings are recommended for UEFI Runtime Drivers. They
+use the runtime specific version of the BaseCryptLib and the null version of the
+TlsLib because TLS services are not typically used at runtime.
 
 ```
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
@@ -388,13 +388,13 @@ TlsLib because TLS services are not typically used in runtime.
 ### PCD Configuration Settings
 
 There are 2 PCD settings that are used to configure cryptographic services.
-`PcdHashApiLibPolicy` is used to configure the hash algorithm provided by the
+`PcdHashApiLibPolicy` is used to configure the hash algorithms provided by the
 BaseHashApiLib library instance. `PcdCryptoServiceFamilyEnable` is used to
 configure the cryptographic services supported by the CryptoPei, CryptoDxe,
 and CryptoSmm modules.
 
 * `gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy` - This PCD indicates the
-  HASH algorithm to to use in the BaseHashApiLib to calculate hash of data. The
+  HASH algorithms to use in the BaseHashApiLib to calculate hash of data. The
   default hashing algorithm for BaseHashApiLib is set to HASH_ALG_SHA256.
   |  Setting   |    Algorithm     |
   |------------|------------------|
@@ -407,8 +407,8 @@ and CryptoSmm modules.
 * `gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable` - Enable/Disable
    the families and individual services produced by the EDK II Crypto
    Protocols/PPIs. The default is all services disabled. This Structured PCD is
-   associated with `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that defined in
-   `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
+   associated with the `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that is
+   defined in `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
 
    There are three layers of priority that determine if a specific family or
    individual cryptographic service is actually enabled in the CryptoPei,
@@ -420,15 +420,15 @@ and CryptoSmm modules.
       OpensslLib instance linked, then the service is always disabled.
    2) BaseCryptLib instance selection.
       * CryptoPei is always linked with the PeiCryptLib instance of the
-        BaseCryptLib library class. The table above have a column for the
+        BaseCryptLib library class. The table above has a column for the
         PeiCryptLib. If the family or service is blank, then that family or
         service is always disabled.
       * CryptoDxe is always linked with the BaseCryptLib instance of the
-        BaseCryptLib library class. The table above have a column for the
+        BaseCryptLib library class. The table above has a column for the
         BaseCryptLib. If the family or service is blank, then that family or
         service is always disabled.
       * CryptoSmm is always linked with the SmmCryptLib instance of the
-        BaseCryptLib library class. The table above have a column for the
+        BaseCryptLib library class. The table above has a column for the
         SmmCryptLib. If the family or service is blank, then that family or
         service is always disabled.
    3) If a family or service is enabled in the OpensslLib instance and it is
@@ -438,11 +438,11 @@ and CryptoSmm modules.
       bit fields for each family of services. All of the families are disabled
       by default. An entire family of services can be enabled by setting the
       family field to the value `PCD_CRYPTO_SERVICE_ENABLE_FAMILY`. Individual
-      services can be enabled by setting a single service name to `TRUE`.
-      Settings listed later in the DSC file have priority over settings earlier
-      in the DSC file, so it is legal for an entire family to be enabled first
-      and then a few individual services disabled by setting the service name to
-      `FALSE`.
+      services can be enabled by setting a single service name (bit) to `TRUE`.
+      Settings listed later in the DSC file have priority over settings listed
+      earlier in the DSC file, so it is valid for an entire family to be enabled
+      first and then for a few individual services to be disabled by setting
+      those service names to `FALSE`.
 
 #### Common PEI PcdCryptoServiceFamilyEnable Settings
 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95855): https://edk2.groups.io/g/devel/message/95855
Mute This Topic: https://groups.io/mt/94730319/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
Posted by Michael D Kinney 1 year, 5 months ago
Hi Laszlo,

Thank you for all the updates.  One comment below.

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, November 2, 2022 2:37 AM
> To: devel@edk2.groups.io; lersek@redhat.com
> Cc: Zurcher, Christopher <christopher.zurcher@microsoft.com>; Jiang, Guomin <guomin.jiang@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Lu, Xiaoyu1
> <xiaoyu1.lu@intel.com>
> Subject: [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
> 
> Commit 244ce33bdd2f ("CryptoPkg: Add Readme.md", 2022-10-24) had added the
> long-awaited documentation on the dynamic crypto services. Fix some of the
> typos and arguable grammar errors in "Readme.md". A few light
> clarifications are also snuck in.
> 
> Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> Cc: Guomin Jiang <guomin.jiang@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     Best displayed as a word-diff for review, for example with the command
> 
>       git show --word-diff
> 
>     or on the web at
> 
>       https://pagure.io/lersek/edk2/c/a7269f170437?branch=cryptopkg_readme_typos
> 
>  CryptoPkg/Readme.md | 48 ++++++++++----------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/CryptoPkg/Readme.md b/CryptoPkg/Readme.md
> index 946aa1e99e7d..9dd3a951836b 100644
> --- a/CryptoPkg/Readme.md
> +++ b/CryptoPkg/Readme.md
> @@ -39,7 +39,7 @@ provides the smallest overall firmware overhead.
> 
>  ## Statically Linking Cryptographic Services
> 
> -The figure below shows an example of a firmware modules that requires the use of
> +The figure below shows an example of a firmware module that requires the use of
>  cryptographic services. The cryptographic services are provided by three library
>  classes called BaseCryptLib, TlsLib, and HashApiLib. These library classes are
>  implemented using APIs from the OpenSSL project that are abstracted by the
> @@ -49,7 +49,7 @@ full C runtime library for firmware components. Instead, the CryptoPkg includes
>  the smallest subset of services required to build the OpenSSL project in the
>  private library class called IntrinsicLib.
> 
> -The CryptoPkg provides several instances if the BaseCryptLib and OpensslLib with
> +The CryptoPkg provides several instances of the BaseCryptLib and OpensslLib with
>  different cryptographic service features and performance optimizations. The
>  platform developer must select the correct instances based on cryptographic
>  service requirements in each UEFI/PI firmware phase (SEC, PEI, DXE, UEFI,
> @@ -97,9 +97,9 @@ linking is not available for SEC or UEFI RT modules.
> 
>  The EDK II modules/libraries that require cryptographic services use the same
>  BaseCryptLib/TlsLib/HashApiLib APIs. This means no source changes are required
> -to use static linking or dynamic linking. It is a platform configuration options
> -to select static linking or dynamic linking. This choice can be make globally,
> -per firmware module type, or individual modules.
> +to use static linking or dynamic linking. It is a platform configuration option
> +to select static linking or dynamic linking. This choice can be made globally,
> +per firmware module type, or for individual modules.
> 
>  ```
>  +===================+    +===================+     +===================+
> @@ -159,7 +159,7 @@ The table below provides a summary of the supported cryptographic services. It
>  indicates if the family or service is deprecated or recommended to not be used.
>  It also shows which *CryptLib library instances support the family or service.
>  If a cell is blank then the service or family is always disabled and the
> -`PcdCryptoServiceFamilyEnable` settings for that family or service is ignored.
> +`PcdCryptoServiceFamilyEnable` setting for that family or service is ignored.
>  If the cell is not blank, then the service or family is configurable using
>  `PcdCryptoServiceFamilyEnable` as long as the correct OpensslLib or TlsLib is
>  also configured.
> @@ -234,10 +234,10 @@ phases (SEC, PEI, DXE, UEFI, SMM, UEFI RT).
> 
>  The following table can be used to help select the best OpensslLib instance for
>  each phase. The Size column only shows the estimated size increase for a
> -compressed IA32/X64 modules that uses the cryptographic services with
> +compressed IA32/X64 module that uses the cryptographic services with
>  `OpensslLib.inf` as the baseline size. The actual size increase depends on the
>  specific set of enabled cryptographic services. If ECC services are not
> -required, then size can be reduced by using OpensslLib.inf instead of
> +required, then the size can be reduced by using OpensslLib.inf instead of
>  `OpensslLibFull.inf`. Performance optimization requires a size increase.
> 
>  | OpensslLib Instance     | SSL | ECC | Perf Opt | CPU Arch | Size  |
> @@ -371,10 +371,10 @@ settings.
> 
>  ### UEFI Runtime Driver Library Mappings
> 
> -UEFI Runtime Drivers only supports static linking of cryptographic services.
> -The following library mappings are recommended for UEFI Runtime Drivers. It uses
> -the runtime specific version of the BaseCryptLib and the null version of the
> -TlsLib because TLS services are not typically used in runtime.
> +UEFI Runtime Drivers only support static linking of cryptographic services.
> +The following library mappings are recommended for UEFI Runtime Drivers. They
> +use the runtime specific version of the BaseCryptLib and the null version of the
> +TlsLib because TLS services are not typically used at runtime.
> 
>  ```
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> @@ -388,13 +388,13 @@ TlsLib because TLS services are not typically used in runtime.
>  ### PCD Configuration Settings
> 
>  There are 2 PCD settings that are used to configure cryptographic services.
> -`PcdHashApiLibPolicy` is used to configure the hash algorithm provided by the
> +`PcdHashApiLibPolicy` is used to configure the hash algorithms provided by the
>  BaseHashApiLib library instance. `PcdCryptoServiceFamilyEnable` is used to
>  configure the cryptographic services supported by the CryptoPei, CryptoDxe,
>  and CryptoSmm modules.
> 
>  * `gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy` - This PCD indicates the
> -  HASH algorithm to to use in the BaseHashApiLib to calculate hash of data. The
> +  HASH algorithms to use in the BaseHashApiLib to calculate hash of data. The

It is my understanding that the BaseHashApiLib supports at most one algorithm.
Perhaps this is only an attribute of the current implementation.  The declaration
of the PCD values for this PCD could support a bitmask that could allow multiple
algorithm values to be ORed together.  However, the DEC file description does not 
state that.  The example below shows the current implementation that does
not interpret as a bitmask.  In fact, setting more than one algorithm value
would fall through to the default clause.  So you recommend an update to DEC
and implementation to support bitmask?  Or restrict the PCD and implementation
to a single algorithm?

UINTN
EFIAPI
HashApiGetContextSize (
  VOID
  )
{
  switch (PcdGet32 (PcdHashApiLibPolicy)) {
    case HASH_ALG_SHA1:
      return Sha1GetContextSize ();
      break;

    case HASH_ALG_SHA256:
      return Sha256GetContextSize ();
      break;

    case HASH_ALG_SHA384:
      return Sha384GetContextSize ();
      break;

    case HASH_ALG_SHA512:
      return Sha512GetContextSize ();
      break;

    case HASH_ALG_SM3_256:
      return Sm3GetContextSize ();
      break;

    default:
      ASSERT (FALSE);
      return 0;
      break;
  }
}



>    default hashing algorithm for BaseHashApiLib is set to HASH_ALG_SHA256.
>    |  Setting   |    Algorithm     |
>    |------------|------------------|
> @@ -407,8 +407,8 @@ and CryptoSmm modules.
>  * `gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable` - Enable/Disable
>     the families and individual services produced by the EDK II Crypto
>     Protocols/PPIs. The default is all services disabled. This Structured PCD is
> -   associated with `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that defined in
> -   `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
> +   associated with the `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that is
> +   defined in `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
> 
>     There are three layers of priority that determine if a specific family or
>     individual cryptographic service is actually enabled in the CryptoPei,
> @@ -420,15 +420,15 @@ and CryptoSmm modules.
>        OpensslLib instance linked, then the service is always disabled.
>     2) BaseCryptLib instance selection.
>        * CryptoPei is always linked with the PeiCryptLib instance of the
> -        BaseCryptLib library class. The table above have a column for the
> +        BaseCryptLib library class. The table above has a column for the
>          PeiCryptLib. If the family or service is blank, then that family or
>          service is always disabled.
>        * CryptoDxe is always linked with the BaseCryptLib instance of the
> -        BaseCryptLib library class. The table above have a column for the
> +        BaseCryptLib library class. The table above has a column for the
>          BaseCryptLib. If the family or service is blank, then that family or
>          service is always disabled.
>        * CryptoSmm is always linked with the SmmCryptLib instance of the
> -        BaseCryptLib library class. The table above have a column for the
> +        BaseCryptLib library class. The table above has a column for the
>          SmmCryptLib. If the family or service is blank, then that family or
>          service is always disabled.
>     3) If a family or service is enabled in the OpensslLib instance and it is
> @@ -438,11 +438,11 @@ and CryptoSmm modules.
>        bit fields for each family of services. All of the families are disabled
>        by default. An entire family of services can be enabled by setting the
>        family field to the value `PCD_CRYPTO_SERVICE_ENABLE_FAMILY`. Individual
> -      services can be enabled by setting a single service name to `TRUE`.
> -      Settings listed later in the DSC file have priority over settings earlier
> -      in the DSC file, so it is legal for an entire family to be enabled first
> -      and then a few individual services disabled by setting the service name to
> -      `FALSE`.
> +      services can be enabled by setting a single service name (bit) to `TRUE`.
> +      Settings listed later in the DSC file have priority over settings listed
> +      earlier in the DSC file, so it is valid for an entire family to be enabled
> +      first and then for a few individual services to be disabled by setting
> +      those service names to `FALSE`.
> 
>  #### Common PEI PcdCryptoServiceFamilyEnable Settings
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95868): https://edk2.groups.io/g/devel/message/95868
Mute This Topic: https://groups.io/mt/94730319/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
Posted by Laszlo Ersek 1 year, 5 months ago
Hi Mike,

On 11/02/22 17:48, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> Thank you for all the updates.  One comment below.
> 
> Mike
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Wednesday, November 2, 2022 2:37 AM
>> To: devel@edk2.groups.io; lersek@redhat.com
>> Cc: Zurcher, Christopher <christopher.zurcher@microsoft.com>; Jiang, Guomin <guomin.jiang@intel.com>; Wang, Jian J
>> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Lu, Xiaoyu1
>> <xiaoyu1.lu@intel.com>
>> Subject: [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
>>
>> Commit 244ce33bdd2f ("CryptoPkg: Add Readme.md", 2022-10-24) had added the
>> long-awaited documentation on the dynamic crypto services. Fix some of the
>> typos and arguable grammar errors in "Readme.md". A few light
>> clarifications are also snuck in.
>>
>> Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
>> Cc: Guomin Jiang <guomin.jiang@intel.com>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     Best displayed as a word-diff for review, for example with the command
>>
>>       git show --word-diff
>>
>>     or on the web at
>>
>>       https://pagure.io/lersek/edk2/c/a7269f170437?branch=cryptopkg_readme_typos
>>
>>  CryptoPkg/Readme.md | 48 ++++++++++----------
>>  1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/CryptoPkg/Readme.md b/CryptoPkg/Readme.md
>> index 946aa1e99e7d..9dd3a951836b 100644
>> --- a/CryptoPkg/Readme.md
>> +++ b/CryptoPkg/Readme.md
>> @@ -39,7 +39,7 @@ provides the smallest overall firmware overhead.
>>
>>  ## Statically Linking Cryptographic Services
>>
>> -The figure below shows an example of a firmware modules that requires the use of
>> +The figure below shows an example of a firmware module that requires the use of
>>  cryptographic services. The cryptographic services are provided by three library
>>  classes called BaseCryptLib, TlsLib, and HashApiLib. These library classes are
>>  implemented using APIs from the OpenSSL project that are abstracted by the
>> @@ -49,7 +49,7 @@ full C runtime library for firmware components. Instead, the CryptoPkg includes
>>  the smallest subset of services required to build the OpenSSL project in the
>>  private library class called IntrinsicLib.
>>
>> -The CryptoPkg provides several instances if the BaseCryptLib and OpensslLib with
>> +The CryptoPkg provides several instances of the BaseCryptLib and OpensslLib with
>>  different cryptographic service features and performance optimizations. The
>>  platform developer must select the correct instances based on cryptographic
>>  service requirements in each UEFI/PI firmware phase (SEC, PEI, DXE, UEFI,
>> @@ -97,9 +97,9 @@ linking is not available for SEC or UEFI RT modules.
>>
>>  The EDK II modules/libraries that require cryptographic services use the same
>>  BaseCryptLib/TlsLib/HashApiLib APIs. This means no source changes are required
>> -to use static linking or dynamic linking. It is a platform configuration options
>> -to select static linking or dynamic linking. This choice can be make globally,
>> -per firmware module type, or individual modules.
>> +to use static linking or dynamic linking. It is a platform configuration option
>> +to select static linking or dynamic linking. This choice can be made globally,
>> +per firmware module type, or for individual modules.
>>
>>  ```
>>  +===================+    +===================+     +===================+
>> @@ -159,7 +159,7 @@ The table below provides a summary of the supported cryptographic services. It
>>  indicates if the family or service is deprecated or recommended to not be used.
>>  It also shows which *CryptLib library instances support the family or service.
>>  If a cell is blank then the service or family is always disabled and the
>> -`PcdCryptoServiceFamilyEnable` settings for that family or service is ignored.
>> +`PcdCryptoServiceFamilyEnable` setting for that family or service is ignored.
>>  If the cell is not blank, then the service or family is configurable using
>>  `PcdCryptoServiceFamilyEnable` as long as the correct OpensslLib or TlsLib is
>>  also configured.
>> @@ -234,10 +234,10 @@ phases (SEC, PEI, DXE, UEFI, SMM, UEFI RT).
>>
>>  The following table can be used to help select the best OpensslLib instance for
>>  each phase. The Size column only shows the estimated size increase for a
>> -compressed IA32/X64 modules that uses the cryptographic services with
>> +compressed IA32/X64 module that uses the cryptographic services with
>>  `OpensslLib.inf` as the baseline size. The actual size increase depends on the
>>  specific set of enabled cryptographic services. If ECC services are not
>> -required, then size can be reduced by using OpensslLib.inf instead of
>> +required, then the size can be reduced by using OpensslLib.inf instead of
>>  `OpensslLibFull.inf`. Performance optimization requires a size increase.
>>
>>  | OpensslLib Instance     | SSL | ECC | Perf Opt | CPU Arch | Size  |
>> @@ -371,10 +371,10 @@ settings.
>>
>>  ### UEFI Runtime Driver Library Mappings
>>
>> -UEFI Runtime Drivers only supports static linking of cryptographic services.
>> -The following library mappings are recommended for UEFI Runtime Drivers. It uses
>> -the runtime specific version of the BaseCryptLib and the null version of the
>> -TlsLib because TLS services are not typically used in runtime.
>> +UEFI Runtime Drivers only support static linking of cryptographic services.
>> +The following library mappings are recommended for UEFI Runtime Drivers. They
>> +use the runtime specific version of the BaseCryptLib and the null version of the
>> +TlsLib because TLS services are not typically used at runtime.
>>
>>  ```
>>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
>> @@ -388,13 +388,13 @@ TlsLib because TLS services are not typically used in runtime.
>>  ### PCD Configuration Settings
>>
>>  There are 2 PCD settings that are used to configure cryptographic services.
>> -`PcdHashApiLibPolicy` is used to configure the hash algorithm provided by the
>> +`PcdHashApiLibPolicy` is used to configure the hash algorithms provided by the
>>  BaseHashApiLib library instance. `PcdCryptoServiceFamilyEnable` is used to
>>  configure the cryptographic services supported by the CryptoPei, CryptoDxe,
>>  and CryptoSmm modules.
>>
>>  * `gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy` - This PCD indicates the
>> -  HASH algorithm to to use in the BaseHashApiLib to calculate hash of data. The
>> +  HASH algorithms to use in the BaseHashApiLib to calculate hash of data. The
> 
> It is my understanding that the BaseHashApiLib supports at most one algorithm.
> Perhaps this is only an attribute of the current implementation.  The declaration
> of the PCD values for this PCD could support a bitmask that could allow multiple
> algorithm values to be ORed together.  However, the DEC file description does not 
> state that.  The example below shows the current implementation that does
> not interpret as a bitmask.  In fact, setting more than one algorithm value
> would fall through to the default clause.  So you recommend an update to DEC
> and implementation to support bitmask?  Or restrict the PCD and implementation
> to a single algorithm?
> 
> UINTN
> EFIAPI
> HashApiGetContextSize (
>   VOID
>   )
> {
>   switch (PcdGet32 (PcdHashApiLibPolicy)) {
>     case HASH_ALG_SHA1:
>       return Sha1GetContextSize ();
>       break;
> 
>     case HASH_ALG_SHA256:
>       return Sha256GetContextSize ();
>       break;
> 
>     case HASH_ALG_SHA384:
>       return Sha384GetContextSize ();
>       break;
> 
>     case HASH_ALG_SHA512:
>       return Sha512GetContextSize ();
>       break;
> 
>     case HASH_ALG_SM3_256:
>       return Sm3GetContextSize ();
>       break;
> 
>     default:
>       ASSERT (FALSE);
>       return 0;
>       break;
>   }
> }

Thank you for confirming this; in fact I had my doubts on this hunk. I
had checked the PCD in the DEC file, but not the implementation.

The DEC file does employ the term "algorithm" in singular, multiple
times at that. However, the individual values are not consecutive --
they are values of distinct bits. This suggests a a bitmap, and it also
seemed "more sensible" for a library to support multiple hash algorithms
at the same time (similarly to how PcdCryptoServiceFamilyEnable could
enable multiple algorithms at the same time). I didn't realize the PCD
was supposed to bind the library to one particular hash algorithm --
that seemed too limiting for an entire platform. (Even with
component-scope or component-type-scope Fixed-at-Build overrides in a
DSC file, it was hard to find a use case for fixating different sets of
modules in the same platform to different *unique* hash algos at build
time.) In effect I expected PcdHashApiLibPolicy to work similarly to
PcdCryptoServiceFamilyEnable.

Additionally -- I had missed @ValidList in the DEC file; sorry about
that. The explicit listing there makes it a bit clearer that
PcdHashApiLibPolicy is not a bitmap.

I don't know why the current values suggest the possibility of a bitmap
-- were they meant as future-proofing, for an implementation that might
support more algos at the same time?

So my suggestion would be to "restrict the PCD and implementation to a
single algorithm" (as you write), in particular remove the confusion by
renumbering the identifiers so they form a consecutive sequence.

Except... :) I notice that:

(a) These macros (such as HASH_ALG_SHA256) are defined in
"MdePkg/Include/IndustryStandard/Tpm20.h". So it's not possible to just
change the integer values in the DEC file but keep the macro names; that
would de-sync the PCD file with "Tpm20.h".

To be honest I find it suboptimal that the Hash library API is bound to
TPM2 specifics -- hashing is more general than its usage in TPM2.

(b) "git blame" actually provides context:

- commit 3feea54eae33 ("CryptoPkg/BaseHashApiLib: Implement Unified Hash
Calculation API", 2020-02-03), for bugzilla 2151, introduced HashApiLib
with a consecutive list of integer hash algo identifiers,

- commit c70bdf9d4a6a ("CryptoPkg/BaseHashApiLib: Align BaseHashApiLib
with TPM 2.0 Implementation", 2020-02-19), for bugzilla 2511, replaced
the originally consecutive values with the TPM2 bit values,

So what I thought was logical had actually existed in the past, but then
it got replaced with the current "bitmask-suggesting" values, for bug 2511.


In the end I think I should drop this hunk from the "Readme.md" patch;
however, while technically correct, the DEC file remains confusing. The
values seem to be aligned with the TPM2 spec just for simplicity's sake,
and that's fine, but it should be documented. IIUC the TPM2 spec does
actually rely on these values being organized into a bitmap, but that's
something that HashApiLib does not utilize. I think this should be
emphasized in the DEC file (i.e. that the library interface permits a
single hash algo only).

We also have:

> @Prompt Set policy for hashing unsigned image for Secure Boot.

which I think is too restrictive; surely there are uses for HashApiLib
other than SecureBoot. This very "Readme.md" file discusses HashApiLib
in the SEC and PEI phases, and Secure Boot is not defined in those phases.

I'm happy to update this patch for Readme.md (note: there's still a typo
to fix in this context: "to to use"); I'd rather leave the
"CryptoPkg.dec" update to someone else :) (Most likely Amol -- CC'd.)

Thanks,
Laszlo


> 
> 
> 
>>    default hashing algorithm for BaseHashApiLib is set to HASH_ALG_SHA256.
>>    |  Setting   |    Algorithm     |
>>    |------------|------------------|
>> @@ -407,8 +407,8 @@ and CryptoSmm modules.
>>  * `gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable` - Enable/Disable
>>     the families and individual services produced by the EDK II Crypto
>>     Protocols/PPIs. The default is all services disabled. This Structured PCD is
>> -   associated with `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that defined in
>> -   `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
>> +   associated with the `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that is
>> +   defined in `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
>>
>>     There are three layers of priority that determine if a specific family or
>>     individual cryptographic service is actually enabled in the CryptoPei,
>> @@ -420,15 +420,15 @@ and CryptoSmm modules.
>>        OpensslLib instance linked, then the service is always disabled.
>>     2) BaseCryptLib instance selection.
>>        * CryptoPei is always linked with the PeiCryptLib instance of the
>> -        BaseCryptLib library class. The table above have a column for the
>> +        BaseCryptLib library class. The table above has a column for the
>>          PeiCryptLib. If the family or service is blank, then that family or
>>          service is always disabled.
>>        * CryptoDxe is always linked with the BaseCryptLib instance of the
>> -        BaseCryptLib library class. The table above have a column for the
>> +        BaseCryptLib library class. The table above has a column for the
>>          BaseCryptLib. If the family or service is blank, then that family or
>>          service is always disabled.
>>        * CryptoSmm is always linked with the SmmCryptLib instance of the
>> -        BaseCryptLib library class. The table above have a column for the
>> +        BaseCryptLib library class. The table above has a column for the
>>          SmmCryptLib. If the family or service is blank, then that family or
>>          service is always disabled.
>>     3) If a family or service is enabled in the OpensslLib instance and it is
>> @@ -438,11 +438,11 @@ and CryptoSmm modules.
>>        bit fields for each family of services. All of the families are disabled
>>        by default. An entire family of services can be enabled by setting the
>>        family field to the value `PCD_CRYPTO_SERVICE_ENABLE_FAMILY`. Individual
>> -      services can be enabled by setting a single service name to `TRUE`.
>> -      Settings listed later in the DSC file have priority over settings earlier
>> -      in the DSC file, so it is legal for an entire family to be enabled first
>> -      and then a few individual services disabled by setting the service name to
>> -      `FALSE`.
>> +      services can be enabled by setting a single service name (bit) to `TRUE`.
>> +      Settings listed later in the DSC file have priority over settings listed
>> +      earlier in the DSC file, so it is valid for an entire family to be enabled
>> +      first and then for a few individual services to be disabled by setting
>> +      those service names to `FALSE`.
>>
>>  #### Common PEI PcdCryptoServiceFamilyEnable Settings
>>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95894): https://edk2.groups.io/g/devel/message/95894
Mute This Topic: https://groups.io/mt/94730319/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
Posted by Michael D Kinney 1 year, 5 months ago
Hi Laszlo,

I think we should consider expending the meaning of PCD to allow
multiple algos and update lib implementation to support multiple.

This should not have a size impact for platforms that only want one
algo.  And should also be backwards compatible to expand meaning to
support multiple.

Let's go with your updated patch that leaves it single algo to get
the Readme.md fixed now.  We can enter new BZs feature requests to
expand PCD from single to multiple and expand lib to support multiple.

Thanks,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Thursday, November 3, 2022 7:35 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Zurcher, Christopher <christopher.zurcher@microsoft.com>; Jiang, Guomin <guomin.jiang@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Sukerkar, Amol N
> <amol.n.sukerkar@intel.com>
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
> 
> Hi Mike,
> 
> On 11/02/22 17:48, Kinney, Michael D wrote:
> > Hi Laszlo,
> >
> > Thank you for all the updates.  One comment below.
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Wednesday, November 2, 2022 2:37 AM
> >> To: devel@edk2.groups.io; lersek@redhat.com
> >> Cc: Zurcher, Christopher <christopher.zurcher@microsoft.com>; Jiang, Guomin <guomin.jiang@intel.com>; Wang, Jian J
> >> <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Lu, Xiaoyu1
> >> <xiaoyu1.lu@intel.com>
> >> Subject: [PATCH] CryptoPkg/Readme.md: typo and grammar fixes
> >>
> >> Commit 244ce33bdd2f ("CryptoPkg: Add Readme.md", 2022-10-24) had added the
> >> long-awaited documentation on the dynamic crypto services. Fix some of the
> >> typos and arguable grammar errors in "Readme.md". A few light
> >> clarifications are also snuck in.
> >>
> >> Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> >> Cc: Guomin Jiang <guomin.jiang@intel.com>
> >> Cc: Jian J Wang <jian.j.wang@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >> Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> >> ---
> >>
> >> Notes:
> >>     Best displayed as a word-diff for review, for example with the command
> >>
> >>       git show --word-diff
> >>
> >>     or on the web at
> >>
> >>       https://pagure.io/lersek/edk2/c/a7269f170437?branch=cryptopkg_readme_typos
> >>
> >>  CryptoPkg/Readme.md | 48 ++++++++++----------
> >>  1 file changed, 24 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/CryptoPkg/Readme.md b/CryptoPkg/Readme.md
> >> index 946aa1e99e7d..9dd3a951836b 100644
> >> --- a/CryptoPkg/Readme.md
> >> +++ b/CryptoPkg/Readme.md
> >> @@ -39,7 +39,7 @@ provides the smallest overall firmware overhead.
> >>
> >>  ## Statically Linking Cryptographic Services
> >>
> >> -The figure below shows an example of a firmware modules that requires the use of
> >> +The figure below shows an example of a firmware module that requires the use of
> >>  cryptographic services. The cryptographic services are provided by three library
> >>  classes called BaseCryptLib, TlsLib, and HashApiLib. These library classes are
> >>  implemented using APIs from the OpenSSL project that are abstracted by the
> >> @@ -49,7 +49,7 @@ full C runtime library for firmware components. Instead, the CryptoPkg includes
> >>  the smallest subset of services required to build the OpenSSL project in the
> >>  private library class called IntrinsicLib.
> >>
> >> -The CryptoPkg provides several instances if the BaseCryptLib and OpensslLib with
> >> +The CryptoPkg provides several instances of the BaseCryptLib and OpensslLib with
> >>  different cryptographic service features and performance optimizations. The
> >>  platform developer must select the correct instances based on cryptographic
> >>  service requirements in each UEFI/PI firmware phase (SEC, PEI, DXE, UEFI,
> >> @@ -97,9 +97,9 @@ linking is not available for SEC or UEFI RT modules.
> >>
> >>  The EDK II modules/libraries that require cryptographic services use the same
> >>  BaseCryptLib/TlsLib/HashApiLib APIs. This means no source changes are required
> >> -to use static linking or dynamic linking. It is a platform configuration options
> >> -to select static linking or dynamic linking. This choice can be make globally,
> >> -per firmware module type, or individual modules.
> >> +to use static linking or dynamic linking. It is a platform configuration option
> >> +to select static linking or dynamic linking. This choice can be made globally,
> >> +per firmware module type, or for individual modules.
> >>
> >>  ```
> >>  +===================+    +===================+     +===================+
> >> @@ -159,7 +159,7 @@ The table below provides a summary of the supported cryptographic services. It
> >>  indicates if the family or service is deprecated or recommended to not be used.
> >>  It also shows which *CryptLib library instances support the family or service.
> >>  If a cell is blank then the service or family is always disabled and the
> >> -`PcdCryptoServiceFamilyEnable` settings for that family or service is ignored.
> >> +`PcdCryptoServiceFamilyEnable` setting for that family or service is ignored.
> >>  If the cell is not blank, then the service or family is configurable using
> >>  `PcdCryptoServiceFamilyEnable` as long as the correct OpensslLib or TlsLib is
> >>  also configured.
> >> @@ -234,10 +234,10 @@ phases (SEC, PEI, DXE, UEFI, SMM, UEFI RT).
> >>
> >>  The following table can be used to help select the best OpensslLib instance for
> >>  each phase. The Size column only shows the estimated size increase for a
> >> -compressed IA32/X64 modules that uses the cryptographic services with
> >> +compressed IA32/X64 module that uses the cryptographic services with
> >>  `OpensslLib.inf` as the baseline size. The actual size increase depends on the
> >>  specific set of enabled cryptographic services. If ECC services are not
> >> -required, then size can be reduced by using OpensslLib.inf instead of
> >> +required, then the size can be reduced by using OpensslLib.inf instead of
> >>  `OpensslLibFull.inf`. Performance optimization requires a size increase.
> >>
> >>  | OpensslLib Instance     | SSL | ECC | Perf Opt | CPU Arch | Size  |
> >> @@ -371,10 +371,10 @@ settings.
> >>
> >>  ### UEFI Runtime Driver Library Mappings
> >>
> >> -UEFI Runtime Drivers only supports static linking of cryptographic services.
> >> -The following library mappings are recommended for UEFI Runtime Drivers. It uses
> >> -the runtime specific version of the BaseCryptLib and the null version of the
> >> -TlsLib because TLS services are not typically used in runtime.
> >> +UEFI Runtime Drivers only support static linking of cryptographic services.
> >> +The following library mappings are recommended for UEFI Runtime Drivers. They
> >> +use the runtime specific version of the BaseCryptLib and the null version of the
> >> +TlsLib because TLS services are not typically used at runtime.
> >>
> >>  ```
> >>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> >> @@ -388,13 +388,13 @@ TlsLib because TLS services are not typically used in runtime.
> >>  ### PCD Configuration Settings
> >>
> >>  There are 2 PCD settings that are used to configure cryptographic services.
> >> -`PcdHashApiLibPolicy` is used to configure the hash algorithm provided by the
> >> +`PcdHashApiLibPolicy` is used to configure the hash algorithms provided by the
> >>  BaseHashApiLib library instance. `PcdCryptoServiceFamilyEnable` is used to
> >>  configure the cryptographic services supported by the CryptoPei, CryptoDxe,
> >>  and CryptoSmm modules.
> >>
> >>  * `gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy` - This PCD indicates the
> >> -  HASH algorithm to to use in the BaseHashApiLib to calculate hash of data. The
> >> +  HASH algorithms to use in the BaseHashApiLib to calculate hash of data. The
> >
> > It is my understanding that the BaseHashApiLib supports at most one algorithm.
> > Perhaps this is only an attribute of the current implementation.  The declaration
> > of the PCD values for this PCD could support a bitmask that could allow multiple
> > algorithm values to be ORed together.  However, the DEC file description does not
> > state that.  The example below shows the current implementation that does
> > not interpret as a bitmask.  In fact, setting more than one algorithm value
> > would fall through to the default clause.  So you recommend an update to DEC
> > and implementation to support bitmask?  Or restrict the PCD and implementation
> > to a single algorithm?
> >
> > UINTN
> > EFIAPI
> > HashApiGetContextSize (
> >   VOID
> >   )
> > {
> >   switch (PcdGet32 (PcdHashApiLibPolicy)) {
> >     case HASH_ALG_SHA1:
> >       return Sha1GetContextSize ();
> >       break;
> >
> >     case HASH_ALG_SHA256:
> >       return Sha256GetContextSize ();
> >       break;
> >
> >     case HASH_ALG_SHA384:
> >       return Sha384GetContextSize ();
> >       break;
> >
> >     case HASH_ALG_SHA512:
> >       return Sha512GetContextSize ();
> >       break;
> >
> >     case HASH_ALG_SM3_256:
> >       return Sm3GetContextSize ();
> >       break;
> >
> >     default:
> >       ASSERT (FALSE);
> >       return 0;
> >       break;
> >   }
> > }
> 
> Thank you for confirming this; in fact I had my doubts on this hunk. I
> had checked the PCD in the DEC file, but not the implementation.
> 
> The DEC file does employ the term "algorithm" in singular, multiple
> times at that. However, the individual values are not consecutive --
> they are values of distinct bits. This suggests a a bitmap, and it also
> seemed "more sensible" for a library to support multiple hash algorithms
> at the same time (similarly to how PcdCryptoServiceFamilyEnable could
> enable multiple algorithms at the same time). I didn't realize the PCD
> was supposed to bind the library to one particular hash algorithm --
> that seemed too limiting for an entire platform. (Even with
> component-scope or component-type-scope Fixed-at-Build overrides in a
> DSC file, it was hard to find a use case for fixating different sets of
> modules in the same platform to different *unique* hash algos at build
> time.) In effect I expected PcdHashApiLibPolicy to work similarly to
> PcdCryptoServiceFamilyEnable.
> 
> Additionally -- I had missed @ValidList in the DEC file; sorry about
> that. The explicit listing there makes it a bit clearer that
> PcdHashApiLibPolicy is not a bitmap.
> 
> I don't know why the current values suggest the possibility of a bitmap
> -- were they meant as future-proofing, for an implementation that might
> support more algos at the same time?
> 
> So my suggestion would be to "restrict the PCD and implementation to a
> single algorithm" (as you write), in particular remove the confusion by
> renumbering the identifiers so they form a consecutive sequence.
> 
> Except... :) I notice that:
> 
> (a) These macros (such as HASH_ALG_SHA256) are defined in
> "MdePkg/Include/IndustryStandard/Tpm20.h". So it's not possible to just
> change the integer values in the DEC file but keep the macro names; that
> would de-sync the PCD file with "Tpm20.h".
> 
> To be honest I find it suboptimal that the Hash library API is bound to
> TPM2 specifics -- hashing is more general than its usage in TPM2.
> 
> (b) "git blame" actually provides context:
> 
> - commit 3feea54eae33 ("CryptoPkg/BaseHashApiLib: Implement Unified Hash
> Calculation API", 2020-02-03), for bugzilla 2151, introduced HashApiLib
> with a consecutive list of integer hash algo identifiers,
> 
> - commit c70bdf9d4a6a ("CryptoPkg/BaseHashApiLib: Align BaseHashApiLib
> with TPM 2.0 Implementation", 2020-02-19), for bugzilla 2511, replaced
> the originally consecutive values with the TPM2 bit values,
> 
> So what I thought was logical had actually existed in the past, but then
> it got replaced with the current "bitmask-suggesting" values, for bug 2511.
> 
> 
> In the end I think I should drop this hunk from the "Readme.md" patch;
> however, while technically correct, the DEC file remains confusing. The
> values seem to be aligned with the TPM2 spec just for simplicity's sake,
> and that's fine, but it should be documented. IIUC the TPM2 spec does
> actually rely on these values being organized into a bitmap, but that's
> something that HashApiLib does not utilize. I think this should be
> emphasized in the DEC file (i.e. that the library interface permits a
> single hash algo only).
> 
> We also have:
> 
> > @Prompt Set policy for hashing unsigned image for Secure Boot.
> 
> which I think is too restrictive; surely there are uses for HashApiLib
> other than SecureBoot. This very "Readme.md" file discusses HashApiLib
> in the SEC and PEI phases, and Secure Boot is not defined in those phases.
> 
> I'm happy to update this patch for Readme.md (note: there's still a typo
> to fix in this context: "to to use"); I'd rather leave the
> "CryptoPkg.dec" update to someone else :) (Most likely Amol -- CC'd.)
> 
> Thanks,
> Laszlo
> 
> 
> >
> >
> >
> >>    default hashing algorithm for BaseHashApiLib is set to HASH_ALG_SHA256.
> >>    |  Setting   |    Algorithm     |
> >>    |------------|------------------|
> >> @@ -407,8 +407,8 @@ and CryptoSmm modules.
> >>  * `gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable` - Enable/Disable
> >>     the families and individual services produced by the EDK II Crypto
> >>     Protocols/PPIs. The default is all services disabled. This Structured PCD is
> >> -   associated with `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that defined in
> >> -   `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
> >> +   associated with the `PCD_CRYPTO_SERVICE_FAMILY_ENABLE` structure that is
> >> +   defined in `Include/Pcd/PcdCryptoServiceFamilyEnable.h`.
> >>
> >>     There are three layers of priority that determine if a specific family or
> >>     individual cryptographic service is actually enabled in the CryptoPei,
> >> @@ -420,15 +420,15 @@ and CryptoSmm modules.
> >>        OpensslLib instance linked, then the service is always disabled.
> >>     2) BaseCryptLib instance selection.
> >>        * CryptoPei is always linked with the PeiCryptLib instance of the
> >> -        BaseCryptLib library class. The table above have a column for the
> >> +        BaseCryptLib library class. The table above has a column for the
> >>          PeiCryptLib. If the family or service is blank, then that family or
> >>          service is always disabled.
> >>        * CryptoDxe is always linked with the BaseCryptLib instance of the
> >> -        BaseCryptLib library class. The table above have a column for the
> >> +        BaseCryptLib library class. The table above has a column for the
> >>          BaseCryptLib. If the family or service is blank, then that family or
> >>          service is always disabled.
> >>        * CryptoSmm is always linked with the SmmCryptLib instance of the
> >> -        BaseCryptLib library class. The table above have a column for the
> >> +        BaseCryptLib library class. The table above has a column for the
> >>          SmmCryptLib. If the family or service is blank, then that family or
> >>          service is always disabled.
> >>     3) If a family or service is enabled in the OpensslLib instance and it is
> >> @@ -438,11 +438,11 @@ and CryptoSmm modules.
> >>        bit fields for each family of services. All of the families are disabled
> >>        by default. An entire family of services can be enabled by setting the
> >>        family field to the value `PCD_CRYPTO_SERVICE_ENABLE_FAMILY`. Individual
> >> -      services can be enabled by setting a single service name to `TRUE`.
> >> -      Settings listed later in the DSC file have priority over settings earlier
> >> -      in the DSC file, so it is legal for an entire family to be enabled first
> >> -      and then a few individual services disabled by setting the service name to
> >> -      `FALSE`.
> >> +      services can be enabled by setting a single service name (bit) to `TRUE`.
> >> +      Settings listed later in the DSC file have priority over settings listed
> >> +      earlier in the DSC file, so it is valid for an entire family to be enabled
> >> +      first and then for a few individual services to be disabled by setting
> >> +      those service names to `FALSE`.
> >>
> >>  #### Common PEI PcdCryptoServiceFamilyEnable Settings
> >>
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95898): https://edk2.groups.io/g/devel/message/95898
Mute This Topic: https://groups.io/mt/94730319/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-