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

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

From: Laszlo Ersek <lersek@redhat.com>

To: David Woodhouse <dwmw2@infradead.org>

Date: 2014-05-23 00:43:57

On 05/21/14 14:19, David Woodhouse wrote:
> On Tue, 2014-05-13 at 04:54 +0200, Laszlo Ersek wrote:
>>
>> But, second, we've known for quite some time that windows server 2008 r2
>> works well with the SeaBIOS CSM and a few edk2 patches (thanks again to
>> David Woodhouse and Kevin O'Connor).
>>
>> There are two problems with the CSM, one specific (and possibly
>> accidental/fixable), and one generic (and inherent).
>>
>> - The specific (possibly accidental, fixable) problem is that using the
>> SeaBIOS CSM plus the CSM infrastructure in edk2 breaks OVMF's S3 resume.
>> I had not set out to track this down because of the other problem (see
>> below).
> 
> Wimp :)

Okay... I did look into it today and I don't know what to say.

Apparently now it Just Works (TM).

Environment:
- host: RHEL-7
- OVMF: SVN r15542 (which includes the patch in this thread),
        aka git a145e28d
- CSM: SeaBIOS git 0784d04c
- guest: Windows 2008 R2 SP1
- guest OS video driver: QXL

I actually performed two tests (two OVMF builds).
(a) In the first OVMF build, I left QemuVideoDxe in.

    QemuVideoDxe grabs the card before VideoDxe could (the order is
    unspecified with the current state of the tree, and this is how it
    has always turned out).

    Before SVN r15540 (== the patch in this thread; aka git 90803342)
    this used to break the default VGA driver of the windows guest (so
    we were forced to evict QemuVideoDxe from the build), but with this
    patch (ie. the VBE shim) in place, it is not the case any more.

(b) In the second OVMF build, I evicted QemuVideoDxe, guaranteeing that
    VideoDxe would grab the card.

S3 suspend/resume worked in both cases.

So let's see why I've been under the impression that it wouldn't.

(1) Jordan committed what we thought would be the last S3 patch as SVN
r15308 (git 34511266), on March 4th:
OvmfPkg: Add DebugAgentLib for Library class mapping for DXE_DRIVER



In that thread we touched on the S3+CSM topic, and I said

  Last time I tested: there was no love lost between CSM and S3,
  independently of video card / driver :) The resume boot path decayed
  to a reboot.

Implying that I didn't retest S3+CSM even at SVN r15308 (git 34511266).
(Side note: at whichever earlier point I had tested it, the CSM build at
that time had meant case (b), unconditionally.)

(2) Additionally, the following patches have been committed since, ie.
in the SVN r15308..r15542 range (using git hashes):

git log --oneline --reverse 34511266..a145e28d -- OvmfPkg/ | cat -n

   1  da78c88 OvmfPkg: raise DXEFV size to 8 MB

unrelated build fix

   2  de5ae37 OvmfPkg: BDS: remove historic (now defunct) boot mode hack
   3  1c9135a OvmfPkg: BDS: QemuBootOrder: don't leak unreferenced boot
              options

series with a fix for the QEMU paravirt boot order, unrelated

   4  c4341e3 OvmfPkg: non-null PcdLib instance for GraphicsConsoleDxe
   5  732295d OvmfPkg: introduce gOvmfPlatformConfigGuid
   6  d945a8b OvmfPkg: introduce empty PlatformDxe
   7  5267c89 OvmfPkg: PlatformDxe: utility functions for saving /
              loading configuration
   8  bdaf30e OvmfPkg: PlatformDxe: set preferred video resolution from
              platform config
   9  877a4db OvmfPkg: PlatformDxe: add an empty HII form
  10  276a7ea OvmfPkg: PlatformDxe: introduce state for the main form
  11  92e7455 OvmfPkg: PlatformDxe: add form widgets for video modes
  12  bc4c536 OvmfPkg/PlatformDxe: Silence warning seen with GCC48 IA32
  13  9c08bbe OvmfPkg: QemuVideoDxe: serialize Start() against callbacks
  14  da07afa OvmfPkg: PlatformDxe: get available resolutions from GOP
  15  1df57ba OvmfPkg: PlatformDxe: add save and discard buttons to the
              form
  16  cbd08bc OvmfPkg: PlatformDxe: connect ExtractConfig() to platform
              data
  17  ddb2c49 OvmfPkg: PlatformDxe: connect RouteConfig() to platform
              data

platform driver series, unrelated

  18  0e8a31f OvmfPkg: PlatformPei: lifecycle fixes for the LockBox area
  19  209c392 OvmfPkg: AcpiS3SaveDxe: do not load if S3 is
              unsupported/disabled in qemu

this series has no effect when QEMU supports S3 (it targeted the S3
unsupported/disabled configuration), hence unrelated

  20  96bbdbc OvmfPkg: AcpiPlatformDxe: download ACPI tables from QEMU

unrelated

  21  e04cca1 OvmfPkg: non-null PcdLib instance for the CSM VideoDxe

only a build fixup, unrelated

  22  3f4b148 OvmfPkg: add a catch-all match for PCI devices in the
              OpenFirmware path

pertains to QEMU paravirt boot order, unrelated

  23  ad43bc6 OvmfPkg: PlatformPei: protect SEC's GUIDed section handler
              table thru S3

this is my suspect

  24  9080334 OvmfPkg: QemuVideoDxe: Int10h stub for Windows 7 & 2008
              (stdvga, QXL)

not related to S3

  25  6b23d76 OvmfPkg/SMBIOS: Reuse handles supplied by underlying VM
  26  a145e28 OvmfPkg/SMBIOS: Add QEMU support to OVMF SMBIOS driver

unrelated

(3) So, my summary is:
- S3+CSM might have *already* worked at SVN r15308 (git 34511266), when
  Jordan committed what we thought was going to be the last
  suspend/resume patch, *or*
- when I later fixed the (hopefully last!) "memory corruption on
  resume" issue in patch 23 above (SVN r15433, aka git ad43bc6), I
  might have unwittingly fixed S3+CSM too.

So now it works apparently, and you might want to spend time only
confirming / retesting it, not fixing it :)

In any case, the rest of my argument for this commit stands:

>> - The generic / inherent problem with the current SeaBIOS CSM + the
>> edk2 CSM infrastructure is that the combination is a nightmare to
>> trace, debug, and modify, for mere mortals. It switches from long
>> mode to real mode and vice versa, and it's impossible to follow
>> unless you know the relevant magic from the Intel SDM from memory.

This patch (SVN r15540 / git 90803342) enables downstreams to ship just
a pure-UEFI build, without having to worry about Windows 7 / Windows
2008 R2.

Thanks
Laszlo


------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel