Hi Laszlo,
On 9/17/19 9:49 PM, Laszlo Ersek wrote:
> According to the UEFI spec -- and to the edk2 header
> "MdePkg/Include/Protocol/EdidOverride.h" too --,
> EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID takes an (EFI_HANDLE*), and not an
> EFI_HANDLE, as second parameter ("ChildHandle").
>
> This is probably [*] a bug in the UEFI spec. Given that this CSM module
> (VideoDxe) had been used for a long time on physical platforms before it
> was moved to OvmfPkg, keep the current "ChildHandle" argument, just cast
> it explicitly.
>
> [*] The edk2 tree contains no other GetEdid() call, and also no GetEdid()
> implementation.
>
> The edk2-platforms tree contains two GetEdid() calls, at commit
> 022c212167e0, in files
> - "Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c",
> - "Drivers/OptionRomPkg/CirrusLogic5430Dxe/Edid.c".
>
> From these, the first passes an (EFI_HANDLE*) as "ChildHandle", the
> second passes an EFI_HANDLE. It's difficult to draw a conclusion. :/
>
> No functional changes.
>
> (I've also requested a non-normative (informative) clarification for the
> UEFI spec: <https://mantis.uefi.org/mantis/view.php?id=2018>, in the
> direction that matches this patch.)
(EFI_HANDLE*) makes sense to me, but I'd rather wait for the spec
clarification before Acking this patch, I don't want we silent a bug
with this cast.
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: David Woodhouse <dw...@infradead.org>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> ---
>
> Notes:
> build-tested only
>
> OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
> b/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
> index 0640656dba14..995136adee27 100644
> --- a/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
> +++ b/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
> @@ -1402,9 +1402,13 @@ BiosVideoCheckForVbe (
> goto Done;
> }
>
> + //
> + // Cast "ChildHandle" to (EFI_HANDLE*) in order to work around the spec
> bug
> + // in UEFI v2.8, reported as Mantis#2018.
> + //
> Status = EdidOverride->GetEdid (
> EdidOverride,
> - BiosVideoPrivate->Handle,
> + (EFI_HANDLE *) BiosVideoPrivate->Handle,
> &EdidAttributes,
> &EdidOverrideDataSize,
> (UINT8 **) &EdidOverrideDataBlock
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#47857): https://edk2.groups.io/g/devel/message/47857
Mute This Topic: https://groups.io/mt/34180226/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
