[PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

[PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

[G Edhaya Chandran]

Updated the check for montonic count in the case of after restart

>From the UEFI Spec:
"The platform’s monotonic counter is comprised of two parts: the high 32 bits and the low 32 bits.
The low 32-bit value is volatile and is reset to zero on every system reset.
It is increased by 1 on every call to GetNextMonotonicCount().
The high 32-bit value is nonvolatile and is increased by one on
whenever the system resets or the low 32-bit counter overflows."

It was found in one case where the higher 32-bit increased by 2
presumably due to the overflow of lower 32-bit counter.
Update the logic to handle this case and to print a warning.

Please find more details in the ticket: https://bugzilla.tianocore.org/show_bug.cgi?id=2774

Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Carolyn Gjertsen <Carolyn.Gjertsen@amd.com>
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>

Signed-off-by: G Edhaya Chandran<edhaya.chandran@arm.com>
---
 .../MiscBootServicesBBTestFunction.c          | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
index 5d631c16d58b..12703d46f98c 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
@@ -1707,12 +1707,20 @@ GetNextMonotonicCountStep2:
                    TplArray[Index]
                    );
 
-    if (SctRShiftU64 (Count2, 32) == SctRShiftU64 (Count, 32) + 1) {
-      AssertionType = EFI_TEST_ASSERTION_PASSED;
-    } else {
-      AssertionType = EFI_TEST_ASSERTION_FAILED;
-    }
-    StandardLib->RecordAssertion (
+    //The new count of upper 32 bits must be atleast 1 more than the old count.
+    //Pass case: new count is equal to old count + 1
+    if (SctRShiftU64 (Count2, 32) <= SctRShiftU64 (Count, 32)) {
+      AssertionType = EFI_TEST_ASSERTION_FAILED;
+    } else {
+      //If new count is more that old count + 1, then print warning.
+      if (SctRShiftU64 (Count2, 32) > SctRShiftU64 (Count, 32) + 1) {
+        AssertionType = EFI_TEST_ASSERTION_WARNING;
+      } else {
+        //new count == old count + 1
+        AssertionType = EFI_TEST_ASSERTION_PASSED;
+      }
+   }
+   StandardLib->RecordAssertion (
                    StandardLib,
                    AssertionType,
                    Index==0? \
-- 
2.17.1

回复: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

[Gao Jie]

Hi Eday,

The patch looks good to me.

Reviewed-by: Barton Gao <gaojie@byosoft.com.cn>

Thanks
Barton

-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 G Edhaya Chandran
发送时间: 2022年3月3日 16:59
收件人: devel@edk2.groups.io
主题: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

Updated the check for montonic count in the case of after restart

>From the UEFI Spec:
"The platform’s monotonic counter is comprised of two parts: the high 32 bits and the low 32 bits.
The low 32-bit value is volatile and is reset to zero on every system reset.
It is increased by 1 on every call to GetNextMonotonicCount().
The high 32-bit value is nonvolatile and is increased by one on
whenever the system resets or the low 32-bit counter overflows."

It was found in one case where the higher 32-bit increased by 2
presumably due to the overflow of lower 32-bit counter.
Update the logic to handle this case and to print a warning.

Please find more details in the ticket: https://bugzilla.tianocore.org/show_bug.cgi?id=2774

Cc: Barton Gao <gaojie@byosoft.com.cn>
Cc: Carolyn Gjertsen <Carolyn.Gjertsen@amd.com>
Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>

Signed-off-by: G Edhaya Chandran<edhaya.chandran@arm.com>
---
 .../MiscBootServicesBBTestFunction.c          | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
index 5d631c16d58b..12703d46f98c 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c
@@ -1707,12 +1707,20 @@ GetNextMonotonicCountStep2:
                    TplArray[Index]
                    );
 
-    if (SctRShiftU64 (Count2, 32) == SctRShiftU64 (Count, 32) + 1) {
-      AssertionType = EFI_TEST_ASSERTION_PASSED;
-    } else {
-      AssertionType = EFI_TEST_ASSERTION_FAILED;
-    }
-    StandardLib->RecordAssertion (
+    //The new count of upper 32 bits must be atleast 1 more than the old count.
+    //Pass case: new count is equal to old count + 1
+    if (SctRShiftU64 (Count2, 32) <= SctRShiftU64 (Count, 32)) {
+      AssertionType = EFI_TEST_ASSERTION_FAILED;
+    } else {
+      //If new count is more that old count + 1, then print warning.
+      if (SctRShiftU64 (Count2, 32) > SctRShiftU64 (Count, 32) + 1) {
+        AssertionType = EFI_TEST_ASSERTION_WARNING;
+      } else {
+        //new count == old count + 1
+        AssertionType = EFI_TEST_ASSERTION_PASSED;
+      }
+   }
+   StandardLib->RecordAssertion (
                    StandardLib,
                    AssertionType,
                    Index==0? \
-- 
2.17.1

Re: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

[G Edhaya Chandran]

Hi Barton,

    Thank you for the review.
Could you please upstream the patch.

With Warm Regards,
Edhay


> -----Original Message-----
> From: Gao Jie <gaojie@byosoft.com.cn>
> Sent: 08 April 2022 12:03
> To: devel@edk2.groups.io; G Edhaya Chandran <Edhaya.Chandran@arm.com>
> Subject: 回复: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check
> for monotonic count after restart
>
> Hi Eday,
>
> The patch looks good to me.
>
> Reviewed-by: Barton Gao <gaojie@byosoft.com.cn>
>
> Thanks
> Barton
>
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 G Edhaya
> Chandran
> 发送时间: 2022年3月3日 16:59
> 收件人: devel@edk2.groups.io
> 主题: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for
> monotonic count after restart
>
> Updated the check for montonic count in the case of after restart
>
> From the UEFI Spec:
> "The platform’s monotonic counter is comprised of two parts: the high 32 bits
> and the low 32 bits.
> The low 32-bit value is volatile and is reset to zero on every system reset.
> It is increased by 1 on every call to GetNextMonotonicCount().
> The high 32-bit value is nonvolatile and is increased by one on whenever the
> system resets or the low 32-bit counter overflows."
>
> It was found in one case where the higher 32-bit increased by 2 presumably
> due to the overflow of lower 32-bit counter.
> Update the logic to handle this case and to print a warning.
>
> Please find more details in the ticket:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2774
>
> Cc: Barton Gao <gaojie@byosoft.com.cn>
> Cc: Carolyn Gjertsen <Carolyn.Gjertsen@amd.com>
> Cc: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
>
> Signed-off-by: G Edhaya Chandran<edhaya.chandran@arm.com>
> ---
>  .../MiscBootServicesBBTestFunction.c          | 20 +++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/Mi
> scBootServicesBBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/Mi
> scBootServicesBBTestFunction.c
> index 5d631c16d58b..12703d46f98c 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/Mi
> scBootServicesBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/Bl
> +++ ackBoxTest/MiscBootServicesBBTestFunction.c
> @@ -1707,12 +1707,20 @@ GetNextMonotonicCountStep2:
>                     TplArray[Index]
>                     );
>
> -    if (SctRShiftU64 (Count2, 32) == SctRShiftU64 (Count, 32) + 1) {
> -      AssertionType = EFI_TEST_ASSERTION_PASSED;
> -    } else {
> -      AssertionType = EFI_TEST_ASSERTION_FAILED;
> -    }
> -    StandardLib->RecordAssertion (
> +    //The new count of upper 32 bits must be atleast 1 more than the old count.
> +    //Pass case: new count is equal to old count + 1
> +    if (SctRShiftU64 (Count2, 32) <= SctRShiftU64 (Count, 32)) {
> +      AssertionType = EFI_TEST_ASSERTION_FAILED;
> +    } else {
> +      //If new count is more that old count + 1, then print warning.
> +      if (SctRShiftU64 (Count2, 32) > SctRShiftU64 (Count, 32) + 1) {
> +        AssertionType = EFI_TEST_ASSERTION_WARNING;
> +      } else {
> +        //new count == old count + 1
> +        AssertionType = EFI_TEST_ASSERTION_PASSED;
> +      }
> +   }
> +   StandardLib->RecordAssertion (
>                     StandardLib,
>                     AssertionType,
>                     Index==0? \
> --
> 2.17.1
>
>
>
> 
>
>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [edk2-devel] [PATCH 1/1] UEFI-SCT: SctPkg: Updated the check for monotonic count after restart

[G Edhaya Chandran]

[-- Attachment #1: Type: text/plain, Size: 136 bytes --]

The patch is upstreamed through the commit-id: https://github.com/tianocore/edk2-test/commit/dedfd87f76a9b48bfc03511c19beba285c4f5a81

[-- Attachment #2: Type: text/html, Size: 140 bytes --]