public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation
@ 2022-06-29 15:59 Dimitrije Pavlov
  2022-07-14 14:18 ` Sunny Wang
  2022-07-19  8:29 ` [edk2-devel] " G Edhaya Chandran
  0 siblings, 2 replies; 4+ messages in thread
From: Dimitrije Pavlov @ 2022-06-29 15:59 UTC (permalink / raw)
  To: devel
  Cc: G Edhaya Chandran, Jeff Booher-Kaeding, Samer El-Haj-Mahmoud,
	Sunny Wang, Jeremy Linton

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation
  2022-06-29 15:59 [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation Dimitrije Pavlov
@ 2022-07-14 14:18 ` Sunny Wang
  2022-07-19  8:29 ` [edk2-devel] " G Edhaya Chandran
  1 sibling, 0 replies; 4+ messages in thread
From: Sunny Wang @ 2022-07-14 14:18 UTC (permalink / raw)
  To: Dimitrije Pavlov, devel@edk2.groups.io
  Cc: G Edhaya Chandran, Jeff Booher-Kaeding, Samer El-Haj-Mahmoud,
	Jeremy Linton, Chandni Cherukuri, 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@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.

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation
  2022-06-29 15:59 [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation Dimitrije Pavlov
  2022-07-14 14:18 ` Sunny Wang
@ 2022-07-19  8:29 ` G Edhaya Chandran
  2022-07-19  8:44   ` G Edhaya Chandran
  1 sibling, 1 reply; 4+ messages in thread
From: G Edhaya Chandran @ 2022-07-19  8:29 UTC (permalink / raw)
  To: Dimitrije Pavlov, devel

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

Reviewed-by: G Edhaya Chandran <edhaya.chandran@arm.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation
  2022-07-19  8:29 ` [edk2-devel] " G Edhaya Chandran
@ 2022-07-19  8:44   ` G Edhaya Chandran
  0 siblings, 0 replies; 4+ messages in thread
From: G Edhaya Chandran @ 2022-07-19  8:44 UTC (permalink / raw)
  To: G Edhaya Chandran, devel

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

Upstreamed through commit : https://github.com/tianocore/edk2-test/commit/4a25c3b3c79f63bd9f98b4fffcb21b5c66dd14bb

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-19  8:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-29 15:59 [edk2-test][PATCH v1 1/1] uefi-sct/SctPkg: Don't always check PixelInformation Dimitrije Pavlov
2022-07-14 14:18 ` Sunny Wang
2022-07-19  8:29 ` [edk2-devel] " G Edhaya Chandran
2022-07-19  8:44   ` G Edhaya Chandran

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox