From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web10.28002.1606093255285918191 for ; Sun, 22 Nov 2020 17:00:57 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 23 Nov 2020 09:00:50 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Laszlo Ersek'" , "'Yao, Jiewen'" , , "'Wang, Jian J'" , "'Mistry, Nishant C'" Cc: , "'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> <9e66694e-a9ca-2916-2b3b-984d430924a5@redhat.com> In-Reply-To: <9e66694e-a9ca-2916-2b3b-984d430924a5@redhat.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIXSBTZWN1cml0eVBrZzogQWRkIFJQTUMgSW5kZXggdG8gdGhlIFJwbWNMaWI=?= Date: Mon, 23 Nov 2020 09:00:50 +0800 Message-ID: <01b601d6c134$163599c0$42a0cd40$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQLDCQW5h9LemGh3lNy4fUTd8kAVyQGGQFvOApQqmkoBicNa/QG4YRZtAkx+O6wBsGjwuqehicBg Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Thanks for your comments. If no other comments, I will sent the patch to re= vert this patch.=20 Thanks Liming > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Laszlo Ersek > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B411=E6=9C=8820=E6=97= =A5 19:02 > =E6=94=B6=E4=BB=B6=E4=BA=BA: Yao, Jiewen ; devel@e= dk2.groups.io; > gaoliming@byosoft.com.cn; Wang, Jian J ; Mistry, > Nishant C > =E6=8A=84=E9=80=81: afish@apple.com; 'Leif Lindholm' = ; Kinney, Michael > D > =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index= to the RpmcLib >=20 > On 11/20/20 08:11, Yao, Jiewen wrote: > > I agree with Liming. > > > > I recommend we follow the code-freeze process. > > =09"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 kee= p > old API and define RequestMonotonicCounterEx(), > IncrementMonotonicCounterEx(). > > > > I am also thinking that we should check in together with a lib consume= r to > show the design to see what is really needed for the counter index. > > > > So I vote to revert the change. >=20 > I agree. Without knowing many of the details: >=20 > The patch references > , and that ticket > is a TianoCore Feature Request. Additionally, comment#0 on the BZ says: >=20 > "Two more features are needed to be added to non-volatile variable > services [...] This BZ is for RPMC based solution only". >=20 > I think the patch should not have been committed. >=20 > Thanks > Laszlo >=20 > > > >> -----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; Mistr= y, > >> Nishant C > >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' > >> ; Kinney, Michael D > >> Subject: =E5=9B=9E=E5=A4=8D: [edk2-devel] [PATCH] SecurityPkg: Add RP= MC 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 thin= k this > >> is new feature. > >> > >> But, this is just my idea. I would like to collect more feedback fro= m the > >> mail list. > >> > >> Thanks > >> Liming > >>> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > >>> =E5=8F=91=E4=BB=B6=E4=BA=BA: Wang, Jian J > >>> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B411=E6=9C=8820=E6= =97=A5 14:33 > >>> =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; gaoliming@byosoft= .com.cn; Mistry, > >> Nishant C > >>> > >>> =E4=B8=BB=E9=A2=98: RE: [edk2-devel] [PATCH] SecurityPkg: Add RPMC I= ndex to the > >> RpmcLib > >>> > >>> Liming, > >>> > >>> Sorry, I didn't notice it. But the patch was just updating the exist= ing > >> 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: =E5=9B=9E=E5=A4=8D: [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 s= oft > >>>> 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 > >>>>> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > >>>>> =E5=8F=91=E4=BB=B6=E4=BA=BA: bounce+27952+67669+4905953+8761045@gr= oups.io > >>>>> =E4=BB=A3=E8=A1=A8 = Wang, > >>> Jian J > >>>>> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B411=E6=9C=8818= =E6=97=A5 11:35 > >>>>> =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; Mistry, Nishant= C > >>>>> > >>>>> =E4=B8=BB=E9=A2=98: 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=3D2594 > >>>>>> > >>>>>> The re-design requires multiple RPMC counter usages. > >>>>>> The consumer will be capable of selecting amongst multiple counte= rs. > >>>>>> > >>>>>> 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 > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >> > >> > >> > >> > >> > >>=20 > >> > >