* [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib @ 2020-11-11 18:49 Nishant Mistry 2020-11-18 3:35 ` [edk2-devel] " Wang, Jian J 0 siblings, 1 reply; 11+ messages in thread From: Nishant Mistry @ 2020-11-11 18:49 UTC (permalink / raw) To: devel 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 <nishant.c.mistry@intel.com> --- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-11 18:49 [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib Nishant Mistry @ 2020-11-18 3:35 ` Wang, Jian J 2020-11-20 6:26 ` 回复: " gaoliming 2020-11-20 7:38 ` Yao, Jiewen 0 siblings, 2 replies; 11+ messages in thread From: Wang, Jian J @ 2020-11-18 3:35 UTC (permalink / raw) To: devel@edk2.groups.io, Mistry, Nishant C Reviewed-by: Jian J Wang <jian.j.wang@intel.com> Regards, Jian > -----Original Message----- > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > --- > 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 > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-18 3:35 ` [edk2-devel] " Wang, Jian J @ 2020-11-20 6:26 ` gaoliming 2020-11-20 6:32 ` Wang, Jian J 2020-11-20 7:38 ` Yao, Jiewen 1 sibling, 1 reply; 11+ messages in thread From: gaoliming @ 2020-11-20 6:26 UTC (permalink / raw) To: devel, jian.j.wang, 'Mistry, Nishant C'; +Cc: gaoliming 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 > <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, Jian J > 发送时间: 2020年11月18日 11:35 > 收件人: devel@edk2.groups.io; Mistry, Nishant C > <nishant.c.mistry@intel.com> > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > Regards, > Jian > > > -----Original Message----- > > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > --- > > 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 > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-20 6:26 ` 回复: " gaoliming @ 2020-11-20 6:32 ` Wang, Jian J 2020-11-20 6:54 ` 回复: " gaoliming 0 siblings, 1 reply; 11+ messages in thread From: Wang, Jian J @ 2020-11-20 6:32 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, Mistry, Nishant C 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 <devel@edk2.groups.io> On Behalf Of gaoliming > Sent: Friday, November 20, 2020 2:27 PM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > Nishant C <nishant.c.mistry@intel.com> > 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 > > <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, Jian J > > 发送时间: 2020年11月18日 11:35 > > 收件人: devel@edk2.groups.io; Mistry, Nishant C > > <nishant.c.mistry@intel.com> > > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib > > > > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > > > Regards, > > Jian > > > > > -----Original Message----- > > > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > > --- > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-20 6:32 ` Wang, Jian J @ 2020-11-20 6:54 ` gaoliming 2020-11-20 7:11 ` Yao, Jiewen 0 siblings, 1 reply; 11+ messages in thread From: gaoliming @ 2020-11-20 6:54 UTC (permalink / raw) To: 'Wang, Jian J', devel, 'Mistry, Nishant C' Cc: afish, lersek, 'Leif Lindholm', michael.d.kinney 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 <jian.j.wang@intel.com> > 发送时间: 2020年11月20日 14:33 > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, Nishant C > <nishant.c.mistry@intel.com> > 主题: 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 <devel@edk2.groups.io> On Behalf Of > gaoliming > > Sent: Friday, November 20, 2020 2:27 PM > > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > > Nishant C <nishant.c.mistry@intel.com> > > 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 > > > <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, > Jian J > > > 发送时间: 2020年11月18日 11:35 > > > 收件人: devel@edk2.groups.io; Mistry, Nishant C > > > <nishant.c.mistry@intel.com> > > > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > RpmcLib > > > > > > > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > > > > > Regards, > > > Jian > > > > > > > -----Original Message----- > > > > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > > > --- > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-20 6:54 ` 回复: " gaoliming @ 2020-11-20 7:11 ` Yao, Jiewen 2020-11-20 11:02 ` Laszlo Ersek 0 siblings, 1 reply; 11+ messages in thread From: Yao, Jiewen @ 2020-11-20 7:11 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, Wang, Jian J, Mistry, Nishant C Cc: afish@apple.com, lersek@redhat.com, 'Leif Lindholm', Kinney, Michael D 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. Thank you Yao Jiewen > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > gaoliming > Sent: Friday, November 20, 2020 2:55 PM > To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Mistry, > Nishant C <nishant.c.mistry@intel.com> > Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' > <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com> > 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 <jian.j.wang@intel.com> > > 发送时间: 2020年11月20日 14:33 > > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, > Nishant C > > <nishant.c.mistry@intel.com> > > 主题: 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 <devel@edk2.groups.io> On Behalf Of > > gaoliming > > > Sent: Friday, November 20, 2020 2:27 PM > > > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > > > Nishant C <nishant.c.mistry@intel.com> > > > 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 > > > > <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, > > Jian J > > > > 发送时间: 2020年11月18日 11:35 > > > > 收件人: devel@edk2.groups.io; Mistry, Nishant C > > > > <nishant.c.mistry@intel.com> > > > > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > > RpmcLib > > > > > > > > > > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > > > > > > > Regards, > > > > Jian > > > > > > > > > -----Original Message----- > > > > > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > > > > --- > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-20 7:11 ` Yao, Jiewen @ 2020-11-20 11:02 ` Laszlo Ersek 2020-11-23 1:00 ` 回复: " gaoliming 0 siblings, 1 reply; 11+ messages in thread From: Laszlo Ersek @ 2020-11-20 11:02 UTC (permalink / raw) 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 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 <https://bugzilla.tianocore.org/show_bug.cgi?id=2594>, 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 <devel@edk2.groups.io> On Behalf Of >> gaoliming >> Sent: Friday, November 20, 2020 2:55 PM >> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Mistry, >> Nishant C <nishant.c.mistry@intel.com> >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' >> <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com> >> 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 <jian.j.wang@intel.com> >>> 发送时间: 2020年11月20日 14:33 >>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, >> Nishant C >>> <nishant.c.mistry@intel.com> >>> 主题: 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 <devel@edk2.groups.io> On Behalf Of >>> gaoliming >>>> Sent: Friday, November 20, 2020 2:27 PM >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, >>>> Nishant C <nishant.c.mistry@intel.com> >>>> 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 >>>>> <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, >>> Jian J >>>>> 发送时间: 2020年11月18日 11:35 >>>>> 收件人: devel@edk2.groups.io; Mistry, Nishant C >>>>> <nishant.c.mistry@intel.com> >>>>> 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the >>> RpmcLib >>>>> >>>>> >>>>> Reviewed-by: Jian J Wang <jian.j.wang@intel.com> >>>>> >>>>> Regards, >>>>> Jian >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> >>>>>> --- >>>>>> 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 >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >> >> >> >> >> >> >> > ^ permalink raw reply [flat|nested] 11+ messages in thread
* 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-20 11:02 ` Laszlo Ersek @ 2020-11-23 1:00 ` gaoliming 2020-11-23 5:23 ` Wang, Jian J 0 siblings, 1 reply; 11+ messages in thread From: gaoliming @ 2020-11-23 1:00 UTC (permalink / raw) To: 'Laszlo Ersek', 'Yao, Jiewen', devel, 'Wang, Jian J', 'Mistry, Nishant C' Cc: afish, 'Leif Lindholm', 'Kinney, Michael D' Thanks for your comments. If no other comments, I will sent the patch to revert this patch. Thanks Liming > -----邮件原件----- > 发件人: Laszlo Ersek <lersek@redhat.com> > 发送时间: 2020年11月20日 19:02 > 收件人: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; > gaoliming@byosoft.com.cn; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > Nishant C <nishant.c.mistry@intel.com> > 抄送: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael > D <michael.d.kinney@intel.com> > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib > > 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 > <https://bugzilla.tianocore.org/show_bug.cgi?id=2594>, 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 <devel@edk2.groups.io> On Behalf Of > >> gaoliming > >> Sent: Friday, November 20, 2020 2:55 PM > >> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Mistry, > >> Nishant C <nishant.c.mistry@intel.com> > >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' > >> <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com> > >> 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 <jian.j.wang@intel.com> > >>> 发送时间: 2020年11月20日 14:33 > >>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, > >> Nishant C > >>> <nishant.c.mistry@intel.com> > >>> 主题: 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 <devel@edk2.groups.io> On Behalf Of > >>> gaoliming > >>>> Sent: Friday, November 20, 2020 2:27 PM > >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; > Mistry, > >>>> Nishant C <nishant.c.mistry@intel.com> > >>>> 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 > >>>>> <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, > >>> Jian J > >>>>> 发送时间: 2020年11月18日 11:35 > >>>>> 收件人: devel@edk2.groups.io; Mistry, Nishant C > >>>>> <nishant.c.mistry@intel.com> > >>>>> 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > >>> RpmcLib > >>>>> > >>>>> > >>>>> Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > >>>>> > >>>>> Regards, > >>>>> Jian > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > >>>>>> --- > >>>>>> 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 > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> > >>>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >> > >> > >> > >> > >> > >> > >> > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-23 1:00 ` 回复: " gaoliming @ 2020-11-23 5:23 ` Wang, Jian J 2020-11-23 6:58 ` Yao, Jiewen 0 siblings, 1 reply; 11+ messages in thread From: Wang, Jian J @ 2020-11-23 5:23 UTC (permalink / raw) To: gaoliming, 'Laszlo Ersek', Yao, Jiewen, devel@edk2.groups.io, Mistry, Nishant C Cc: afish@apple.com, 'Leif Lindholm', Kinney, Michael D Liming, I'm ok to revert the patch. Thanks doing it. Jiewen, Currently the RpmcLib has no real users in edk2 master. I don't see reasons to keep old interface. Besides, new interface can be used for one RPMC use case (e.g. defining a default RPMC index), even if variable protection feature just needs one eventually. From potential design change due to security consideration, I prefer to just keep new interface to reduce the impact. Regards, Jian > -----Original Message----- > From: gaoliming <gaoliming@byosoft.com.cn> > Sent: Monday, November 23, 2020 9:01 AM > To: 'Laszlo Ersek' <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, Nishant C > <nishant.c.mistry@intel.com> > Cc: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > RpmcLib > > Thanks for your comments. If no other comments, I will sent the patch to revert > this patch. > > Thanks > Liming > > -----邮件原件----- > > 发件人: Laszlo Ersek <lersek@redhat.com> > > 发送时间: 2020年11月20日 19:02 > > 收件人: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; > > gaoliming@byosoft.com.cn; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > > Nishant C <nishant.c.mistry@intel.com> > > 抄送: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael > > D <michael.d.kinney@intel.com> > > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib > > > > 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 > > <https://bugzilla.tianocore.org/show_bug.cgi?id=2594>, 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 <devel@edk2.groups.io> On Behalf Of > > >> gaoliming > > >> Sent: Friday, November 20, 2020 2:55 PM > > >> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Mistry, > > >> Nishant C <nishant.c.mistry@intel.com> > > >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' > > >> <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com> > > >> 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 <jian.j.wang@intel.com> > > >>> 发送时间: 2020年11月20日 14:33 > > >>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, > > >> Nishant C > > >>> <nishant.c.mistry@intel.com> > > >>> 主题: 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 <devel@edk2.groups.io> On Behalf Of > > >>> gaoliming > > >>>> Sent: Friday, November 20, 2020 2:27 PM > > >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; > > Mistry, > > >>>> Nishant C <nishant.c.mistry@intel.com> > > >>>> 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 > > >>>>> <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, > > >>> Jian J > > >>>>> 发送时间: 2020年11月18日 11:35 > > >>>>> 收件人: devel@edk2.groups.io; Mistry, Nishant C > > >>>>> <nishant.c.mistry@intel.com> > > >>>>> 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > > >>> RpmcLib > > >>>>> > > >>>>> > > >>>>> Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > >>>>> > > >>>>> Regards, > > >>>>> Jian > > >>>>> > > >>>>>> -----Original Message----- > > >>>>>> From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > >>>>>> --- > > >>>>>> 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 > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-23 5:23 ` Wang, Jian J @ 2020-11-23 6:58 ` Yao, Jiewen 0 siblings, 0 replies; 11+ messages in thread From: Yao, Jiewen @ 2020-11-23 6:58 UTC (permalink / raw) To: Wang, Jian J, gaoliming, 'Laszlo Ersek', devel@edk2.groups.io, Mistry, Nishant C Cc: afish@apple.com, 'Leif Lindholm', Kinney, Michael D Hi Jian I tend to agree with you. My point is: we need review the RPMC lib again with a real use case, before we change the API. So far, I am not sure adding CountIndex is the best idea. I think the current design is incomplete, with the fact that there is no GetCounterNumber() API. On the other hand, if you think there is no real use case and we are free to update the API, then why not remove the RpmcLib.h at first? We can add it back with a solid solution later. Thank you Yao Jiewen > -----Original Message----- > From: Wang, Jian J <jian.j.wang@intel.com> > Sent: Monday, November 23, 2020 1:23 PM > To: gaoliming <gaoliming@byosoft.com.cn>; 'Laszlo Ersek' > <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; > devel@edk2.groups.io; Mistry, Nishant C <nishant.c.mistry@intel.com> > Cc: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael D > <michael.d.kinney@intel.com> > Subject: RE: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > RpmcLib > > Liming, > I'm ok to revert the patch. Thanks doing it. > > Jiewen, > Currently the RpmcLib has no real users in edk2 master. I don't see reasons > to keep old interface. Besides, new interface can be used for one RPMC use > case (e.g. defining a default RPMC index), even if variable protection feature > just needs one eventually. From potential design change due to security > consideration, I prefer to just keep new interface to reduce the impact. > > Regards, > Jian > > > -----Original Message----- > > From: gaoliming <gaoliming@byosoft.com.cn> > > Sent: Monday, November 23, 2020 9:01 AM > > To: 'Laszlo Ersek' <lersek@redhat.com>; Yao, Jiewen > <jiewen.yao@intel.com>; > > devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > Nishant C > > <nishant.c.mistry@intel.com> > > Cc: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, Michael > D > > <michael.d.kinney@intel.com> > > Subject: 回复: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > > RpmcLib > > > > Thanks for your comments. If no other comments, I will sent the patch to > revert > > this patch. > > > > Thanks > > Liming > > > -----邮件原件----- > > > 发件人: Laszlo Ersek <lersek@redhat.com> > > > 发送时间: 2020年11月20日 19:02 > > > 收件人: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; > > > gaoliming@byosoft.com.cn; Wang, Jian J <jian.j.wang@intel.com>; Mistry, > > > Nishant C <nishant.c.mistry@intel.com> > > > 抄送: afish@apple.com; 'Leif Lindholm' <leif@nuviainc.com>; Kinney, > Michael > > > D <michael.d.kinney@intel.com> > > > 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > RpmcLib > > > > > > 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 > > > <https://bugzilla.tianocore.org/show_bug.cgi?id=2594>, 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 <devel@edk2.groups.io> On Behalf Of > > > >> gaoliming > > > >> Sent: Friday, November 20, 2020 2:55 PM > > > >> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; > Mistry, > > > >> Nishant C <nishant.c.mistry@intel.com> > > > >> Cc: afish@apple.com; lersek@redhat.com; 'Leif Lindholm' > > > >> <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com> > > > >> 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 <jian.j.wang@intel.com> > > > >>> 发送时间: 2020年11月20日 14:33 > > > >>> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Mistry, > > > >> Nishant C > > > >>> <nishant.c.mistry@intel.com> > > > >>> 主题: 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 <devel@edk2.groups.io> On Behalf Of > > > >>> gaoliming > > > >>>> Sent: Friday, November 20, 2020 2:27 PM > > > >>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; > > > Mistry, > > > >>>> Nishant C <nishant.c.mistry@intel.com> > > > >>>> 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 > > > >>>>> <bounce+27952+67669+4905953+8761045@groups.io> 代表 Wang, > > > >>> Jian J > > > >>>>> 发送时间: 2020年11月18日 11:35 > > > >>>>> 收件人: devel@edk2.groups.io; Mistry, Nishant C > > > >>>>> <nishant.c.mistry@intel.com> > > > >>>>> 主题: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > > > >>> RpmcLib > > > >>>>> > > > >>>>> > > > >>>>> Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > > >>>>> > > > >>>>> Regards, > > > >>>>> Jian > > > >>>>> > > > >>>>>> -----Original Message----- > > > >>>>>> From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > > >>>>>> --- > > > >>>>>> 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 > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> > > > >>>> > > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib 2020-11-18 3:35 ` [edk2-devel] " Wang, Jian J 2020-11-20 6:26 ` 回复: " gaoliming @ 2020-11-20 7:38 ` Yao, Jiewen 1 sibling, 0 replies; 11+ messages in thread From: Yao, Jiewen @ 2020-11-20 7:38 UTC (permalink / raw) To: devel@edk2.groups.io, Wang, Jian J, Mistry, Nishant C Hey The more I review the code, the more I think we should revisit the usage mode for multiple counters. Some thought: 1) In previous design, it is very clear that we only have one counter. We read it and we increase it. But here, if we add Index, that means there could be multiple counters. Then the question is how many? There is no API to tell us how many counters we can use. For multiple counter case, I think we need at least one API - GetCounterNumber(). 2) The description for Index is not clear. Does that start from 0 or 1, to any ID to match hardware? If it is the last one, then we need a bit-mask, or a valid ID array to show which counter number is valid to use. 3) Compatibility is another problem, if the old solution *just* need one counter, then it has to use the new API to select which counter? Now, I am not sure if using *Index* is the best way to resolve the problem. I recommend we check in a real solution to use the RpmcLib, then we can see what interface is really needed. Thank you Yao Jiewen > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang, > Jian J > Sent: Wednesday, November 18, 2020 11:35 AM > To: devel@edk2.groups.io; Mistry, Nishant C <nishant.c.mistry@intel.com> > Subject: Re: [edk2-devel] [PATCH] SecurityPkg: Add RPMC Index to the > RpmcLib > > > Reviewed-by: Jian J Wang <jian.j.wang@intel.com> > > Regards, > Jian > > > -----Original Message----- > > From: devel@edk2.groups.io <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 <nishant.c.mistry@intel.com> > > --- > > 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 > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-11-23 6:58 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-11 18:49 [PATCH] SecurityPkg: Add RPMC Index to the RpmcLib Nishant Mistry 2020-11-18 3:35 ` [edk2-devel] " Wang, Jian J 2020-11-20 6:26 ` 回复: " gaoliming 2020-11-20 6:32 ` Wang, Jian J 2020-11-20 6:54 ` 回复: " gaoliming 2020-11-20 7:11 ` Yao, Jiewen 2020-11-20 11:02 ` Laszlo Ersek 2020-11-23 1:00 ` 回复: " gaoliming 2020-11-23 5:23 ` Wang, Jian J 2020-11-23 6:58 ` Yao, Jiewen 2020-11-20 7:38 ` Yao, Jiewen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox