From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 19 Jun 2019 14:57:17 -0700 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E1115368B1; Wed, 19 Jun 2019 21:57:07 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-91.ams2.redhat.com [10.36.116.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id BC16A5D9C6; Wed, 19 Jun 2019 21:57:05 +0000 (UTC) Subject: Re: [edk2] [PATCH] OvmfPkg: QemuVideoDxe: Int10h stub for Windows 2008 R2 SP1 (stdvga, QXL) To: David Woodhouse , Mike Maslenkin Cc: "devel@edk2.groups.io" References: <1399571116-26442-1-git-send-email-lersek@redhat.com> <5370CA20.1030504@redhat.com> <1399911663.21359.36.camel@mg-think.sw.ru> <5bb64abf34399bae00b678d0cd8b5df1ac81871b.camel@infradead.org> From: "Laszlo Ersek" Message-ID: Date: Wed, 19 Jun 2019 23:57:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5bb64abf34399bae00b678d0cd8b5df1ac81871b.camel@infradead.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 19 Jun 2019 21:57:13 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi David, On 06/17/19 12:52, David Woodhouse wrote: > On Mon, 2014-05-12 at 20:21 +0400, Mike Maslenkin wrote: >>> >>>>> + Segment0 = 0; >>>>> + Segment0Pages = 1; >>>>> + Status = gBS->AllocatePages (AllocateAddress, EfiReservedMemoryType, >>>>> + Segment0Pages, &Segment0); >>>>> + if (EFI_ERROR (Status)) { >>>>> + goto RestorePam1; >>>>> + } >>>> >>>> If CSM is enabled, we will fail to allocate, right? >> >> Allocation at LegacyBiosInstall() function will fail, but no one cares >> about it and MemoryAddress remains uninitialized. This is because uefi >> video driver is being initialized earlier. > > Right... at the time the above code runs, the CSM has already been > initialised and installed a stub 'iret' handler for INT 10h, which in > my case happens to be at F000:F065. > > Because the CSM chose to put that in the F-segment not the E-segment, > that happens not to trigger the check for an existing handler: > > // > // Check if a video BIOS handler has been installed previously -- we > // shouldn't override a real video BIOS with our shim, nor our own shim if > // it's already present. > // > Handler = (Int0x10->Segment << 4) + Int0x10->Offset; > if (Handler >= SegmentC && Handler < SegmentF) { > DEBUG ((EFI_D_INFO, "%a: Video BIOS handler found at %04x:%04x\n", > __FUNCTION__, Int0x10->Segment, Int0x10->Offset)); > return; > } > > So InstallVbeShim() goes ahead and copies the shim to the C-segment and > points the INT10 vector to it (at C000:0200 it seems). > > Later, LegacyBiosInstallRom() shadows the video OpROM, stomping on the > shim. The very *next* thing it does before actually invoking the newly- > shadowed OpROM is make an INT 10h call to put the display into a plain > text mode. Which blows up since there's nothing useful at C000:0200 any > more. > > > There are a few ways we could fix this... > > If I just move that PrepareToScanRom hook invocation (that sets the > text mode) to happen before the CopyMem() of the shadowing, that makes > things work again. But mostly by luck. > > If I change the check in InstallVbeShim() to be '<= SegmentF' then the > VBE shim won't install itself even over the CSM's iret stub. This is > basically equivalent to making the VBE Shim refuse to install if > CSM_ENABLE is set. And might be the right thing to do, since the VBE > Shim isn't enough to actually make legacy code work. > > It might also work if you were to allocate the space for the VBE shim > so that we don't later try to shadow the real ROM to the same location. > > Or maybe we should be letting the legacy BIOS video driver take > precedence if the CSM has a video BIOS, and not letting the native > drivers bind at all? > In 2013, you submitted the following patch: OvmfPkg: Don't build in QemuVideoDxe when we have CSM The thread starts here: https://www.mail-archive.com/edk2-devel@lists.sourceforge.net/msg01871.html After an update: http://mid.mail-archive.com/1360493281.7383.26.camel@shinybook.infradead.org I had given my R-b: http://mid.mail-archive.com/511816AD.9000603@redhat.com But, the patch was never merged. The commit hash referenced in those messages still works (pointing into your personal repo): http://git.infradead.org/users/dwmw2/edk2.git/commitdiff/22253c949e5 Can you resubmit that patch please? Thanks, Laszlo