Re: [edk2-devel] [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib

["Min Xu" <min.m.xu@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]

Hi, Sami
Please see my comments inline.

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sami Mujawar
Sent: Tuesday, November 2, 2021 5:43 PM
To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH V4 2/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpm2MeasureBootLib


+/**

+  Create CcEvent from Tcg2Event.

+

+  CcEvent is similar to Tcg2Event except the MrIndex.

+

+  @param  CcProtocol  Pointer to the located Cc Measurement protocol instance.

+  @param  Tcg2Event   Pointer to the Tcg2Event.

+  @param  EventSize   Size of the Event.

+  @param  EfiCcEvent  The created CcEvent

+

+  @retval EFI_SUCCESS           Successfully create the CcEvent

+  @retval EFI_INVALID_PARAMETER The input parameter is invalid

+  @retval EFI_UNSUPPORTED       The input PCRIndex cannot be mapped to Cc MR

+  @retval EFI_OUT_OF_RESOURCES  Out of resource

+**/

+EFI_STATUS
[SAMI] Is EFIAPI needed here?
[Min] EFIAPI is not needed here. From the EDKII C Coding Standards Spec (https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/56_declarations_and_types)
“The EFIAPI modifier must be used for all UEFI defined API functions, as well as for any function that takes a variable number of arguments. All protocol functions as well as public functions exposed by drivers must also be declared EFIAPI. This establishes a common calling convention for functions that could be referenced by other code that has potentially been built using a different compiler, with a different native calling convention”
CreateCcEventFromTcg2Event is only called internally and it will not be exposed outside. So EFIAPI is not needed.



+CreateCcEventFromTcg2Event (

+  IN  EFI_CC_MEASUREMENT_PROTOCOL   *CcProtocol,

+  IN  EFI_TCG2_EVENT                *Tcg2Event,

+  IN  UINT32                        EventSize,

+  IN OUT EFI_CC_EVENT               **EfiCcEvent

+  )

+{

+  UINT32            MrIndex;
[SAMI] I think it may be good to use the typedef for the measurment register index here i.e. EFI_CC_MR_INDEX.
[Min] Thanks for reminder. It will be fixed in the next version.

Thanks
Min_._,_._,_

[-- Attachment #2: Type: text/html, Size: 7273 bytes --]