Looks good to me. Thanks for working on this, Dimitrije.
I had an offline discussion with Dimitrije. Either this patch or the patch for
SCT (https://edk2.groups.io/g/devel/topic/92068027) can fix the inconsistent
test failure issue mentioned in
https://bugzilla.tianocore.org/show_bug.cgi?id=3601
Reviewed-by: Sunny Wang <sunny.w...@arm.com>
-----Original Message-----
From: Dimitrije Pavlov <dimitrije.pav...@arm.com>
Sent: 28 June 2022 19:48
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>; Jiewen Yao
<jiewen....@intel.com>; Jordan Justen <jordan.l.jus...@intel.com>; Gerd
Hoffmann <kra...@redhat.com>; Jeff Booher-Kaeding
<jeff.booher-kaed...@arm.com>; Samer El-Haj-Mahmoud
<samer.el-haj-mahm...@arm.com>; Sunny Wang <sunny.w...@arm.com>; Jeremy Linton
<jeremy.lin...@arm.com>
Subject: [PATCH v1 1/1] OvmfPkg/QemuVideoDxe: Zero out PixelInformation in
QueryMode
Ensure that the PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is zeroed out in
EFI_GRAPHICS_OUTPUT_PROTOCOL.QueryMode() and
EFI_GRAPHICS_OUTPUT_PROTOCOL.SetMode() when PixelFormat is
PixelBlueGreenRedReserved8BitPerColor.
According to UEFI 2.9 Section 12.9, PixelInformation field of the
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION structure is valid only if
PixelFormat is PixelBitMask. This means that firmware is not required
to fill out the PixelInformation field for other PixelFormat types,
which implies that the QemuVideoDxe implementation is technically
correct.
However, not zeroing out those fields will leak the contents of the
memory returned by the memory allocator, so it is better to explicitly
set them to zero.
In addition, the SCT test suite relies on PixelInformation always
having a consistent value, which causes failures.
Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: Jeff Booher-Kaeding <jeff.booher-kaed...@arm.com>
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>
Cc: Sunny Wang <sunny.w...@arm.com>
Cc: Jeremy Linton <jeremy.lin...@arm.com>
Signed-off-by: Dimitrije Pavlov <dimitrije.pav...@arm.com>
---
OvmfPkg/QemuVideoDxe/Gop.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 0c4dea7fb6f2..7a9fe208c99c 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -31,7 +31,14 @@ QemuVideoCompleteModeInfo (
Info->PixelInformation.ReservedMask = 0;
} else if (ModeData->ColorDepth == 32) {
DEBUG ((DEBUG_INFO, "PixelBlueGreenRedReserved8BitPerColor\n"));
- Info->PixelFormat = PixelBlueGreenRedReserved8BitPerColor;
+ Info->PixelFormat =
PixelBlueGreenRedReserved8BitPerColor;
+ Info->PixelInformation.RedMask = 0;
+ Info->PixelInformation.GreenMask = 0;
+ Info->PixelInformation.BlueMask = 0;
+ Info->PixelInformation.ReservedMask = 0;
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: Invalid ColorDepth %u", __FUNCTION__,
ModeData->ColorDepth));
+ ASSERT (FALSE);
}
Info->PixelsPerScanLine = Info->HorizontalResolution;
--
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 (#91351): https://edk2.groups.io/g/devel/message/91351
Mute This Topic: https://groups.io/mt/92050521/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
