[edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation

[Sunny Wang]

Looks good to me. Thanks for fixing the issue, Dimitrije.
Add Chandni. She has run into this issue as well.

Reviewed-by: Sunny Wang <sunny.wang at arm.com>

-----Original Message-----
From: Dimitrije Pavlov <Dimitrije.Pavlov at arm.com>
Sent: 29 June 2022 16:59
To: devel at edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran at arm.com>; Jeff Booher-Kaeding <Jeff.Booher-Kaeding at arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>; Sunny Wang <Sunny.Wang at arm.com>; Jeremy Linton <Jeremy.Linton at arm.com>
Subject: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation

According to UEFI 2.9 Section 12.9, the PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is valid only if
PixelFormat is PixelBitMask. The current implementation always checks
the contents of PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure returned by QueryMode,
regardless of PixelFormat. Check PixelInformation only if
PixelFormat is PixelBitMask.

Cc: G Edhaya Chandran <Edhaya.Chandran at arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding at arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud at arm.com>
Cc: Sunny Wang <Sunny.Wang at arm.com>
Cc: Jeremy Linton <Jeremy.Linton at arm.com>

Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov at arm.com>
---
 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestConformance.c | 30 ++++++++++++++------
 uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestFunction.c    | 19 +++++++++----
 2 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestConformance.c
index 13e7227f5845..b2bff9d756b1 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestConformance.c
@@ -493,16 +493,28 @@ Returns:
                                  );
       if (Status != EFI_SUCCESS) {
         AssertionType = EFI_TEST_ASSERTION_FAILED;
-      }   else {
+      } else {
         AssertionType = EFI_TEST_ASSERTION_PASSED;
-      }
-
-      if (SctCompareMem (
-          (void *) info,
-          (void *) GraphicsOutput->Mode->Info,
-          sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION)
-          ) != 0) {
-        AssertionType = EFI_TEST_ASSERTION_FAILED;
+        if (info != NULL) {
+          //
+          // PixelInformation is checked only if PixelFormat is PixelBitMask
+          //
+          if ( info->Version              != GraphicsOutput->Mode->Info->Version
+            || info->HorizontalResolution != GraphicsOutput->Mode->Info->HorizontalResolution
+            || info->VerticalResolution   != GraphicsOutput->Mode->Info->VerticalResolution
+            || info->PixelFormat          != GraphicsOutput->Mode->Info->PixelFormat
+            || info->PixelsPerScanLine    != GraphicsOutput->Mode->Info->PixelsPerScanLine
+            || ( info->PixelFormat == PixelBitMask
+              && ( info->PixelInformation.RedMask      != GraphicsOutput->Mode->Info->PixelInformation.RedMask
+                || info->PixelInformation.GreenMask    != GraphicsOutput->Mode->Info->PixelInformation.GreenMask
+                || info->PixelInformation.BlueMask     != GraphicsOutput->Mode->Info->PixelInformation.BlueMask
+                || info->PixelInformation.ReservedMask != GraphicsOutput->Mode->Info->PixelInformation.ReservedMask)))
+          {
+            AssertionType = EFI_TEST_ASSERTION_FAILED;
+          }
+        } else {
+          AssertionType = EFI_TEST_ASSERTION_FAILED;
+        }
       }

       if (info != NULL) {
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestFunction.c
index da51fbc44596..f31ea8175af8 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/GraphicsOutput/BlackBoxTest/GraphicsOutputBBTestFunction.c
@@ -125,11 +125,20 @@ Returns:
     } else {
       AssertionType = EFI_TEST_ASSERTION_PASSED;
       if (Info != NULL) {
-        if (SctCompareMem (
-            (void *) Info,
-            (void *) GraphicsOutput->Mode->Info,
-            sizeof (EFI_GRAPHICS_OUTPUT_MODE_INFORMATION)
-            ) != 0) {
+        //
+        // PixelInformation is checked only if PixelFormat is PixelBitMask
+        //
+        if ( Info->Version              != GraphicsOutput->Mode->Info->Version
+          || Info->HorizontalResolution != GraphicsOutput->Mode->Info->HorizontalResolution
+          || Info->VerticalResolution   != GraphicsOutput->Mode->Info->VerticalResolution
+          || Info->PixelFormat          != GraphicsOutput->Mode->Info->PixelFormat
+          || Info->PixelsPerScanLine    != GraphicsOutput->Mode->Info->PixelsPerScanLine
+          || ( Info->PixelFormat == PixelBitMask
+            && ( Info->PixelInformation.RedMask      != GraphicsOutput->Mode->Info->PixelInformation.RedMask
+              || Info->PixelInformation.GreenMask    != GraphicsOutput->Mode->Info->PixelInformation.GreenMask
+              || Info->PixelInformation.BlueMask     != GraphicsOutput->Mode->Info->PixelInformation.BlueMask
+              || Info->PixelInformation.ReservedMask != GraphicsOutput->Mode->Info->PixelInformation.ReservedMask)))
+        {
           AssertionType = EFI_TEST_ASSERTION_FAILED;
         }
       } else {
--
2.34.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.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#91350): https://edk2.groups.io/g/devel/message/91350
Mute This Topic: https://groups.io/mt/92068027/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-