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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#42600): https://edk2.groups.io/g/devel/message/42600 Mute This Topic: https://groups.io/mt/32093442/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-