public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [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-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

* 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

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