From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web11.23843.1605870142814983269 for ; Fri, 20 Nov 2020 03:02:23 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QFxim215; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605870141; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YCa46iQG8QB6fBMTD4nG5i4jrGdTHpo5cTC8LbSETeg=; b=QFxim215mI7lURgfjpzL6O/7LDS/TKWmWdg6kZ/rXPKQWfH9QQP3b3ZfcHTiT04wcrFbC0 9HM53/j5gHRQp4QU24IGFx3Z1XIzXMUznNycGxENFDrYvQA5+sa9aJjSUtG8dJErkNQVQS fbcFC4LgY6//LwzbilOTYjIBFsb9aOI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-580-Bt0CdL0VOMOL7LTfvU7yKQ-1; Fri, 20 Nov 2020 06:02:16 -0500 X-MC-Unique: Bt0CdL0VOMOL7LTfvU7yKQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 66F44801B13; Fri, 20 Nov 2020 11:02:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-10.ams2.redhat.com [10.36.115.10]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5AF235D9D0; Fri, 20 Nov 2020 11:02:13 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib To: "Yao, Jiewen" , "devel@edk2.groups.io" , "gaoliming@byosoft.com.cn" , "Wang, Jian J" , "Mistry, Nishant C" Cc: "afish@apple.com" , 'Leif Lindholm' , "Kinney, Michael D" References: <661e44cf628ae7315fc738b64a52736f6b5b5285.1605047447.git.nishant.c.mistry@intel.com> <017b01d6bf06$1558f9f0$400aedd0$@byosoft.com.cn> <017e01d6bf0a$00882970$01987c50$@byosoft.com.cn> From: "Laszlo Ersek" Message-ID: <9e66694e-a9ca-2916-2b3b-984d430924a5@redhat.com> Date: Fri, 20 Nov 2020 12:02:12 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 11/20/20 08:11, Yao, Jiewen wrote: > I agree with Liming. > > I recommend we follow the code-freeze process. > "By the date of the soft feature freeze, developers must have sent their patches to the mailing list and received positive maintainer reviews (Reviewed-by or Acked-by tags)." > > The re-design could be compatible in some way. For example, we can keep old API and define RequestMonotonicCounterEx(), IncrementMonotonicCounterEx(). > > I am also thinking that we should check in together with a lib consumer to show the design to see what is really needed for the counter index. > > So I vote to revert the change. I agree. Without knowing many of the details: The patch references , and that ticket is a TianoCore Feature Request. Additionally, comment#0 on the BZ says: "Two more features are needed to be added to non-volatile variable services [...] This BZ is for RPMC based solution only". I think the patch should not have been committed. Thanks Laszlo > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of >> gaoliming >> Sent: Friday, November 20, 2020 2:55 PM >> To: Wang, Jian J ; devel@edk2.groups.io; Mistry, >> Nishant C >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' >> ; Kinney, Michael D >> Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >> RpmcLib >> >> Jian: >> The commit message mentions that the re-design requires multiple RPMC >> counter usages. >> The library API is also updated to support multiple RPMC. So, I think this >> is new feature. >> >> But, this is just my idea. I would like to collect more feedback from the >> mail list. >> >> Thanks >> Liming >>> -----邮件原件----- >>> 发件人: Wang, Jian J >>> 发送时间: 2020年11月20日 14:33 >>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, >> Nishant C >>> >>> 主题: RE: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >> RpmcLib >>> >>> Liming, >>> >>> Sorry, I didn't notice it. But the patch was just updating the existing >> code. It'd >>> be >>> more like bug fix than feature, I think. >>> >>> Regards, >>> Jian >>> >>>> -----Original Message----- >>>> From: devel@edk2.groups.io On Behalf Of >>> gaoliming >>>> Sent: Friday, November 20, 2020 2:27 PM >>>> To: devel@edk2.groups.io; Wang, Jian J ; Mistry, >>>> Nishant C >>>> Cc: gaoliming@byosoft.com.cn >>>> Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >>>> RpmcLib >>>> >>>> Jian: >>>> This change is like a feature instead of bug fix. Now, we are in soft >>>> feature freeze phase. >>>> According to SFF definition >>>> https://github.com/tianocore/tianocore.github.io/wiki/SoftFeatureFreeze, >>>> this feature should be deferred to next stable tag. >>>> >>>> So, I suggest to revert this change, and merge it after the stable tag >>>> 202011. >>>> >>>> Thanks >>>> Liming >>>>> -----邮件原件----- >>>>> 发件人: bounce+27952+67669+4905953+8761045@groups.io >>>>> 代表 Wang, >>> Jian J >>>>> 发送时间: 2020年11月18日 11:35 >>>>> 收件人: devel@edk2.groups.io; Mistry, Nishant C >>>>> >>>>> 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >>> RpmcLib >>>>> >>>>> >>>>> Reviewed-by: Jian J Wang >>>>> >>>>> Regards, >>>>> Jian >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io On Behalf Of >>> Nishant >>>>>> Mistry >>>>>> Sent: Thursday, November 12, 2020 2:49 AM >>>>>> To: devel@edk2.groups.io >>>>>> Subject: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >>> RpmcLib >>>>>> >>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2594 >>>>>> >>>>>> The re-design requires multiple RPMC counter usages. >>>>>> The consumer will be capable of selecting amongst multiple counters. >>>>>> >>>>>> Signed-off-by: Nishant C Mistry >>>>>> --- >>>>>> SecurityPkg/Include/Library/RpmcLib.h | 6 +++++- >>>>>> SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c | 6 +++++- >>>>>> 2 files changed, 10 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/SecurityPkg/Include/Library/RpmcLib.h >>>>>> b/SecurityPkg/Include/Library/RpmcLib.h >>>>>> index 5882bfae2f..3c15bce1ce 100644 >>>>>> --- a/SecurityPkg/Include/Library/RpmcLib.h >>>>>> +++ b/SecurityPkg/Include/Library/RpmcLib.h >>>>>> @@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>>>> /** >>>>>> Requests the monotonic counter from the designated RPMC >>> counter. >>>>>> >>>>>> + @param[in] CounterIndex The RPMC index >>>>>> @param[out] CounterValue A pointer to a buffer >>> to >>>>> store the RPMC >>>>>> value. >>>>>> >>>>>> @retval EFI_SUCCESS The operation >>> completed >>>>> successfully. >>>>>> @@ -23,12 +24,15 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>>>> EFI_STATUS >>>>>> EFIAPI >>>>>> RequestMonotonicCounter ( >>>>>> + IN UINT8 CounterIndex, >>>>>> OUT UINT32 *CounterValue >>>>>> ); >>>>>> >>>>>> /** >>>>>> Increments the monotonic counter in the SPI flash device by 1. >>>>>> >>>>>> + @param[in] CounterIndex The RPMC index >>>>>> + >>>>>> @retval EFI_SUCCESS The operation >>> completed >>>>> successfully. >>>>>> @retval EFI_DEVICE_ERROR A device error >>> occurred >>>>> while attempting >>>>>> to update the counter. >>>>>> @retval EFI_UNSUPPORTED The operation is >>>>> un-supported. >>>>>> @@ -36,7 +40,7 @@ RequestMonotonicCounter ( >>>>>> EFI_STATUS >>>>>> EFIAPI >>>>>> IncrementMonotonicCounter ( >>>>>> - VOID >>>>>> + IN UINT8 CounterIndex >>>>>> ); >>>>>> >>>>>> #endif >>>>>> diff --git a/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c >>>>>> b/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c >>>>>> index e1dd09eb10..697e493a7c 100644 >>>>>> --- a/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c >>>>>> +++ b/SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c >>>>>> @@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>>>> /** >>>>>> Requests the monotonic counter from the designated RPMC >>> counter. >>>>>> >>>>>> + @param[in] CounterIndex The RPMC index >>>>>> @param[out] CounterValue A pointer to a buffer >>> to >>>>> store the RPMC >>>>>> value. >>>>>> >>>>>> @retval EFI_SUCCESS The operation >>> completed >>>>> successfully. >>>>>> @@ -21,6 +22,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >>>>>> EFI_STATUS >>>>>> EFIAPI >>>>>> RequestMonotonicCounter ( >>>>>> + IN UINT8 CounterIndex, >>>>>> OUT UINT32 *CounterValue >>>>>> ) >>>>>> { >>>>>> @@ -31,6 +33,8 @@ RequestMonotonicCounter ( >>>>>> /** >>>>>> Increments the monotonic counter in the SPI flash device by 1. >>>>>> >>>>>> + @param[in] CounterIndex The RPMC index >>>>>> + >>>>>> @retval EFI_SUCCESS The operation >>> completed >>>>> successfully. >>>>>> @retval EFI_DEVICE_ERROR A device error >>> occurred >>>>> while attempting >>>>>> to update the counter. >>>>>> @retval EFI_UNSUPPORTED The operation is >>>>> un-supported. >>>>>> @@ -38,7 +42,7 @@ RequestMonotonicCounter ( >>>>>> EFI_STATUS >>>>>> EFIAPI >>>>>> IncrementMonotonicCounter ( >>>>>> - VOID >>>>>> + IN UINT8 CounterIndex >>>>>> ) >>>>>> { >>>>>> ASSERT (FALSE); >>>>>> -- >>>>>> 2.16.2.windows.1 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >> >> >> >> >> >> >> >