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

["Sunny Wang" <Sunny.Wang@arm.com>]

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@arm.com>

-----Original Message-----
From: Dimitrije Pavlov <Dimitrije.Pavlov@arm.com>
Sent: 29 June 2022 16:59
To: devel@edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Sunny Wang <Sunny.Wang@arm.com>; Jeremy Linton <Jeremy.Linton@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@arm.com>
Cc: Jeff Booher-Kaeding <Jeff.Booher-Kaeding@arm.com>
Cc: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Sunny Wang <Sunny.Wang@arm.com>
Cc: Jeremy Linton <Jeremy.Linton@arm.com>

Signed-off-by: Dimitrije Pavlov <Dimitrije.Pavlov@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.