SecurityPkg/Include/Library/RpmcLib.h | 6 +++++- SecurityPkg/Library/RpmcLibNull/RpmcLibNull.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-)
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67327): https://edk2.groups.io/g/devel/message/67327
Mute This Topic: https://groups.io/mt/78195042/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67669): https://edk2.groups.io/g/devel/message/67669
Mute This Topic: https://groups.io/mt/78195042/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> >
> >
> >
> >
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67756): https://edk2.groups.io/g/devel/message/67756
Mute This Topic: https://groups.io/mt/78195042/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> >
> >
> >
> >
> >
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67746): https://edk2.groups.io/g/devel/message/67746
Mute This Topic: https://groups.io/mt/78383985/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67748): https://edk2.groups.io/g/devel/message/67748
Mute This Topic: https://groups.io/mt/78384041/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67750): https://edk2.groups.io/g/devel/message/67750
Mute This Topic: https://groups.io/mt/78384166/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
>
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67751): https://edk2.groups.io/g/devel/message/67751
Mute This Topic: https://groups.io/mt/78384368/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>>
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67764): https://edk2.groups.io/g/devel/message/67764
Mute This Topic: https://groups.io/mt/78384368/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67782): https://edk2.groups.io/g/devel/message/67782
Mute This Topic: https://groups.io/mt/78444060/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>>>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67787): https://edk2.groups.io/g/devel/message/67787
Mute This Topic: https://groups.io/mt/78447505/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>>>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67791): https://edk2.groups.io/g/devel/message/67791
Mute This Topic: https://groups.io/mt/78447505/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.