[PATCH v1 0/1] Update Stall_Func interval to avoid IO timeouts

[PATCH v1 0/1] Update Stall_Func interval to avoid IO timeouts

[robwoo]

These changes contain an adjustment to the duration of the stall command
to avoid disk corruption via IO timeouts.

Robert Wood (1):
  MiscBootServices: Stall_Func: Reduces the stall interval for
    Stall_Func

 uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h     | 2 +-
 uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>

-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[Robert Wood]

The Stall_Func test on the highest TPL causes issues with the disk IO by
blocking interrupts. This blocking can cause disk corruption through IO
timeouts. Since this doesn't seem to be the intent of the test this
revision reduces the stall interval from 10 seconds to 4 and adjusts the
delta tolerance in scale.

Signed-off-by: Robert Wood <rwood.ce@comcast.net>

Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h     | 2 +-
 uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestFunction.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
index 9e98ec013c74..4f8eaa4c70ea 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
@@ -46,7 +46,7 @@ typedef struct _RESET_DATA {
  { 0xA6033499, 0xE4AF, 0x44f5, {0x9D, 0x16, 0x30, 0x78, 0xD8, 0x61, 0x32, 0x28 }}
 
 #define TPL_ARRAY_SIZE 3
-#define MAX_SECOND_MARGIN 2
+#define MAX_SECOND_MARGIN 1
 
 //
 // Change size from TPL_ARRAY_SIZE to TPL_ARRAY_SIZE + 1
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 ad72646bada2..f831ed6fbccc 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
@@ -827,7 +827,7 @@ BBTestStallInterfaceTest (
       StartTime = Epoch;
     OldTpl = gtBS->RaiseTPL (TplArray[Index]);
     Status = gtBS->Stall (
-                     10000000
+                     4000000
                      );
     gtBS->RestoreTPL (OldTpl);
     if (gtRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[G Edhaya Chandran]

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

On Fri, Sep 30, 2022 at 05:28 AM, Robert Wood wrote:

> 
> MiscBootServicesBBTestFunction.c

Hi Robert,

Can you please also raise a Bugzilla ticket for this issue here: Bug List (tianocore.org) ( https://bugzilla.tianocore.org/buglist.cgi?component=UEFI-SCT&list_id=22841&product=EDK2%20Test&resolution=--- )
Please do attach the failure logs in the ticket.

This issue was discussed in the forum. May we know how the value of 4000000 was arrived for the new Stall value?

With Warm Regards,
Edhay

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

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[Samer El-Haj-Mahmoud]

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

BZ ticket created: https://bugzilla.tianocore.org/show_bug.cgi?id=4105

Patch was at: https://edk2.groups.io/g/discuss/message/1115


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of G Edhaya Chandran via groups.io
Sent: Thursday, October 6, 2022 10:37 AM
To: Robert Wood <robwoo@google.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

On Fri, Sep 30, 2022 at 05:28 AM, Robert Wood wrote:
MiscBootServicesBBTestFunction.c
Hi Robert,

   Can you please also raise a Bugzilla ticket for this issue here: Bug List (tianocore.org)<https://bugzilla.tianocore.org/buglist.cgi?component=UEFI-SCT&list_id=22841&product=EDK2%20Test&resolution=--->
Please do attach the failure logs in the ticket.

This issue was discussed in the forum. May we know how the value of 4000000 was arrived for the new Stall value?

With Warm Regards,
Edhay

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.

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

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[G Edhaya Chandran]

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

Thank you Samer for creating the ticket on Bugzilla.

The next step by the forum is to build SCT with the patch at devel@edk2.groups.io | [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func ( https://edk2.groups.io/g/devel/topic/94007106#94539 )

and run regression on three reference hardwares. Once verified, we will upstream the patch.

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

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[G Edhaya Chandran]

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

Hi Robert,

In the present patch the verification of elapsed time is failing.
The patch will need additional changes as below (bolded):

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 ad72646b..f3d189bd 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
@@ -827,7 +827,7 @@ BBTestStallInterfaceTest (
StartTime = Epoch;
OldTpl = gtBS->RaiseTPL (TplArray[Index]);
Status = gtBS->Stall (
-                     10000000
+                     4000000^M
);
gtBS->RestoreTPL (OldTpl);
if (gtRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
@@ -845,7 +845,7 @@ BBTestStallInterfaceTest (
(Index == 1? \
gMiscBootServicesBBTestFunctionAssertionGuid021: \
gMiscBootServicesBBTestFunctionAssertionGuid022),
-                   L"BS.Stall - 10 seconds",
*+                   L"BS.Stall - 4 seconds",^M*
L"%a:%d:Status - %r, TPL - %d",
__FILE__,
(UINTN)__LINE__,
@@ -853,8 +853,8 @@ BBTestStallInterfaceTest (
TplArray[Index]
);
SecondsElapsed = GetSecondsElapsed (&StartTime, &EndTime);
-    if ((SecondsElapsed <= 10 + MAX_SECOND_MARGIN) &&
-         (SecondsElapsed >= 10 - MAX_SECOND_MARGIN)) {
*+    if ((SecondsElapsed <= 4 + MAX_SECOND_MARGIN) &&^M*
*+         (SecondsElapsed >= 4 - MAX_SECOND_MARGIN)) {^M*
AssertionType = EFI_TEST_ASSERTION_PASSED;
} else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
index 9e98ec01..4f8eaa4c 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest/MiscBootServicesBBTestMain.h
@@ -46,7 +46,7 @@ typedef struct _RESET_DATA {
{ 0xA6033499, 0xE4AF, 0x44f5, {0x9D, 0x16, 0x30, 0x78, 0xD8, 0x61, 0x32, 0x28 }}

#define TPL_ARRAY_SIZE 3
-#define MAX_SECOND_MARGIN 2
+#define MAX_SECOND_MARGIN 1^M

//
// Change size from TPL_ARRAY_SIZE to TPL_ARRAY_SIZE + 1

I have verified the new changes and it executes fine.
Also uploaded the changes to bugzilla 4105 – SctPkg: Stall_Func test can cause disk timeouts and testing failure (tianocore.org) ( https://bugzilla.tianocore.org/show_bug.cgi?id=4105 )
You could please also check and reply.

With Warm Regards,

Edhay

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

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[Robert Wood]

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

Hi G Edhaya,

Thank you for taking the time to review this patch and issue. I have tested the changes you have made and they pass locally and resolve the issues we are experiencing with this test.

Thanks,
Robert Wood

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

Re: [edk2-devel] [PATCH v1 1/1] MiscBootServices: Stall_Func: Reduces the stall interval for Stall_Func

[G Edhaya Chandran]

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

Hi Robert,

Thank you for the confirmation.
The patch is upstreamed through the below commit: https://github.com/tianocore/edk2-test/commit/1d181ad0d82520c099435ff08a8e829b44f493cc

With Warm Regards,
Edhay

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