Re: [edk2] [PATCH] OvmfPkg: QemuVideoDxe: Int10h stub for Windows 2008 R2 SP1 (stdvga, QXL)

["David Woodhouse" <dwmw2@infradead.org>]

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

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?


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]