On 12/11/13 12:33, Laszlo Ersek wrote:
> I get reliable suspend and resume with this series. Tested the X64 and
> the Ia32X64 builds, with an X64 Fedora 19 guest. Nothing seems to be
> corrupted, X11 makes no difference wrt. stability. The guest even
> withstands repeated suspends and resumes in a row. Ssh connections to
> the guest via virtio-net survive the suspend/resume intact. The display
> does die, but that's something to be figured out later -- it might
> involve guest kernel debugging.

I have great news regarding VGA resume too :)

As I said above, I'm using a Fedora 19 guest for testing (kernel
3.11.10-200.fc19.x86_64). I tested two video devices, QXL and Cirrus.

First, I'll report what I've just learned about framebuffer handling in
Linux (the guest). It may be trivial for others but it took me a few
hours until I noticed this, and it explained my suffering :)

(1) When booting Linux with OVMF's GOP (doesn't matter with which video
card), the first driver grabbing the card is the efifb driver. This is
reported in dmesg as:

[    0.891094] efifb: probing for efifb
[    0.896161] efifb: framebuffer at 0xa4000000, mapped to
               0xffffc90000780000, using 1876k, total 1875k
[    0.906944] efifb: mode is 800x600x32, linelength=3200, pages=1
[    0.913958] efifb: scrolling: redraw
[    0.918211] efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0
[    0.926951] Console: switching to colour frame buffer device 100x37
[    0.935261] fb0: EFI VGA frame buffer device

However, soon after, a card-specific FB driver comes along, and kicks
off the efifb driver. For QXL the driver is called "qxldrmfb", and the
messages look like

[    7.178669] fb: conflicting fb hw usage qxldrmfb vs EFI VGA -
               removing generic driver
[    7.200125] Console: switching to colour dummy device 80x25
[    7.353888] fbcon: qxldrmfb (fb0) is primary device
[    7.379517] Console: switching to colour frame buffer device 100x37
[    7.412263] qxl 0000:00:02.0: fb0: qxldrmfb frame buffer device
[    7.412265] qxl 0000:00:02.0: registered panic notifier
[    7.427977] [drm] Initialized qxl 0.1.0 20120117 for 0000:00:02.0 on
               minor 0

For Cirrus, the driver is called "cirrusdrmfb", and it kicks out efifb
the same way.

If someone wants to prevent this, then blacklisting the given DRM FB
driver in /etc/modprobe.d/ is possible. (However, in that case, X11
support may go away.)

The current FB configuration can be queried with the

  fbset --info

command.

(2) So, let's consider four cases:

- QXL with qxldrmfb
- QXL with efifb (qxldrmfb blacklisted)
- Cirrus with cirrusdrmfb
- Cirrus with efifb (cirrusdrmfb blacklisted)

(2a) QXL with qxldrmfb

Suspend/resume works out of the box with this configuration. The guest
driver probably implements the necessary power management callbacks, and
the display is correctly restored in both console and X11/GUI mode.

One nit that I have is that the character console performance is really
sluggish and the screen updates are ugly. But that's my fault, because I
was too lazy to set up a SPICE display, and QXL is very likely not
optimized for VNC (which is what I used).

So, we have one config at least where the guest user has "everything",
console, GUI, and suspend/resume. (And I didn't have to write one line
of code for it :))


(2b) QXL with efifb (qxldrmfb blacklisted)

I'll be honest: I didn't test this. qxldrmfb just works (TM).


(2c) Cirrus with cirrusdrmfb

This case was killing me. Cirrus is my normal qemu setup, and I haven't
been aware about the efifb/cirrusdrmfb "conflict" until today.

cirrusdrmfb has apparently no support for suspend/resume. The screen
freezes at suspend, and it doesn't reanimate at resume.

I wrote an OVMF bootscript patch (more about it later), and the screen
started to do *something* after resume, but it presented the typical
sheared image of the stride being wrong.

My S3 bootscript restored the original settings used in OVMF, which
correspond to a stride of 3200 bytes (32 bits per pixel, 800 pixels per
line). That's also how efifb initially comes up:

[    0.881039] efifb: mode is 800x600x32, linelength=3200, pages=1

But cirrusdrmfb takes control, and it changes the stride. This is
visible only with the fbset --info command (see at "LineLength"):

  mode "800x600"
      geometry 800 600 800 600 24
      timings 0 0 0 0 0 0 0
      accel true
      rgba 8/16,8/8,8/0,0/0
  endmode

  Frame buffer device information:
      Name        : cirrusdrmfb
      Address     : (nil)
      Size        : 0
      Type        : PACKED PIXELS
      Visual      : TRUECOLOR
      XPanStep    : 1
      YPanStep    : 1
      YWrapStep   : 0
      LineLength  : 2400
      Accelerator : No

So, when my patch configured qemu / the Cirrus device with a stride of
3200 at resume, the emulated hw got out of sync with the cirrusdrmfb
driver, and the screen was messed up.

Maybe this stride could be changed with a module parameter... or I could
dive back in the VGA register dumps in QemuVideoDxe and figure out once
again which magic bit controls the stride.

Right now, cirrusdrmfb supports console (good performance with VNC) and
X11 (ditto), but it doesn't support suspend/resume within the guest.
OVMF's VGA register dumps are also not appropriate for cirrusdrmfb's
2400 stride.


(2d) Cirrus with efifb (cirrusdrmfb blacklisted)

This setup seems to rule out X11 at once (probably not a popular
setup...), but it is useful for testing the bootscript functionality.
The fbset --info command says

  mode "800x600-75"
      # D: 48.001 MHz, H: 46.876 kHz, V: 75.121 Hz
      geometry 800 600 800 600 32
      timings 20833 96 32 16 4 96 4
      rgba 8/16,8/8,8/0,8/24
  endmode

  Frame buffer device information:
      Name        : EFI VGA
      Address     : 0xa0000000
      Size        : 1921024
      Type        : PACKED PIXELS
      Visual      : TRUECOLOR
      XPanStep    : 0
      YPanStep    : 0
      YWrapStep   : 0
      LineLength  : 3200
      Accelerator : No

The 75Hz refresh rate is fake (the efifb driver seems to make up those
timings only for fbset's sake); the register dump in QemuVideoDxe sets a
rate of 60Hz. But it doesn't matter.

The important thing is the stride; efifb preserves OVMF's original
configuration, it only uses the GOP's framebuffer and that's it.

The efifb driver doesn't implement the suspend/resume hooks either. But,
because it leaves OVMF's hardware config intact, *repeating* that
configuration at resume time with a boot script does work -- the screen
is restored to a state that matches efifb's idea of it.

Please the attached patch (to be applied on top of this series).


I haven't investigated different guest resolutions (this is relevant
because OVMF's register dumps configure resolution as well). My guest
kernel command line includes "video=800x600" because I don't actually
want to work in guest windows, they just take up screen real estate.

Summary:
- the future is guest-side suspend/resume for video drivers,
- users can pick qxldrmfb and have GUI with suspend/resume immediately,
- boot scripts do work (as demonstrated with cirrus+efifb), but they
  are not dynamic enough and can't follow guest-side hw reconfig
  (stride & likely resolution changes).

Maybe the guest could write a boot script via SMM, but resume support in
a guest driver is probably better. My attached patch should probably
stay a proof of concept only.

Thanks
Laszlo
From 934954fae02fa801f438eb1cc61456b0d1b70ba9 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <[email protected]>
Date: Thu, 12 Dec 2013 03:40:02 +0100
Subject: [PATCH] OvmfPkg: QemuVideoDxe: preliminary S3 boot script for Cirrus
 VGA

Hook boot script writing into the low-level port-IO functions, and call
InitializeCirrusGraphicsMode() from QemuVideoGraphicsOutputConstructor()
with the boot script hook enabled.

This way the IO traffic is sent to the boot script rather than the device,
for one initial (extra) invocation.

The script does the right thing during resume only if the hardware
configuration has not changed from OVMF's 800x600x32 until suspend.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <[email protected]>
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |  1 +
 OvmfPkg/QemuVideoDxe/Qemu.h           |  2 ++
 OvmfPkg/QemuVideoDxe/Driver.c         | 45 +++++++++++++++++++++++++++++++++++
 OvmfPkg/QemuVideoDxe/Gop.c            | 32 +++++++++++++++++++++++++
 4 files changed, 80 insertions(+)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf 
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index 30284fc..8cc3d72 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -59,6 +59,7 @@
   gEfiGraphicsOutputProtocolGuid                # PROTOCOL BY_START
   gEfiDevicePathProtocolGuid                    # PROTOCOL BY_START
   gEfiPciIoProtocolGuid                         # PROTOCOL TO_START
+  gEfiS3SaveStateProtocolGuid                   # PROTOCOL SOMETIMES_CONSUMED
 
 [Pcd]
   gOptionRomPkgTokenSpaceGuid.PcdDriverSupportedEfiVersion
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index 38d6872..bee8785 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -25,6 +25,7 @@
 #include <Protocol/PciIo.h>
 #include <Protocol/DriverSupportedEfiVersion.h>
 #include <Protocol/DevicePath.h>
+#include <Protocol/S3SaveState.h>
 
 #include <Library/DebugLib.h>
 #include <Library/UefiDriverEntryPoint.h>
@@ -113,6 +114,7 @@ typedef struct {
   UINT8                                 *LineBuffer;
   BOOLEAN                               HardwareNeedsStarting;
   QEMU_VIDEO_VARIANT                    Variant;
+  EFI_S3_SAVE_STATE_PROTOCOL            *BootScript;
 } QEMU_VIDEO_PRIVATE_DATA;
 
 ///
diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c
index 53718e1..547485b 100644
--- a/OvmfPkg/QemuVideoDxe/Driver.c
+++ b/OvmfPkg/QemuVideoDxe/Driver.c
@@ -16,6 +16,7 @@
 
 #include "Qemu.h"
 #include <IndustryStandard/Acpi.h>
+#include <Pi/PiS3BootScript.h>
 
 EFI_DRIVER_BINDING_PROTOCOL gQemuVideoDriverBinding = {
   QemuVideoControllerDriverSupported,
@@ -558,6 +559,17 @@ outb (
   UINT8                           Data
   )
 {
+  if (Private->BootScript != NULL) {
+    Private->BootScript->Write (Private->BootScript,
+                           EFI_BOOT_SCRIPT_IO_WRITE_OPCODE,
+                           EfiBootScriptWidthUint8,
+                           (UINT64) Address,
+                           (UINTN) 1,
+                           &Data
+                           );
+    return;
+  }
+
   Private->PciIo->Io.Write (
                       Private->PciIo,
                       EfiPciIoWidthUint8,
@@ -585,6 +597,17 @@ outw (
   UINT16                          Data
   )
 {
+  if (Private->BootScript != NULL) {
+    Private->BootScript->Write (Private->BootScript,
+                           EFI_BOOT_SCRIPT_IO_WRITE_OPCODE,
+                           EfiBootScriptWidthUint16,
+                           (UINT64) Address,
+                           (UINTN) 1,
+                           &Data
+                           );
+    return;
+  }
+
   Private->PciIo->Io.Write (
                       Private->PciIo,
                       EfiPciIoWidthUint16,
@@ -612,6 +635,22 @@ inb (
 {
   UINT8 Data;
 
+  Data = 0;
+  if (Private->BootScript != NULL) {
+    //
+    // Implement read by issuing one poll that succeeds immediately.
+    //
+    Private->BootScript->Write (Private->BootScript,
+                           EFI_BOOT_SCRIPT_IO_POLL_OPCODE,
+                           EfiBootScriptWidthUint8,
+                           (UINT64) Address,
+                           (VOID *) &Data, // polling exit value
+                           (VOID *) &Data, // polling mask
+                           (UINT64) 1      // # of attempts
+                           );
+    return Data;
+  }
+
   Private->PciIo->Io.Read (
                       Private->PciIo,
                       EfiPciIoWidthUint8,
@@ -640,6 +679,8 @@ inw (
 {
   UINT16  Data;
 
+  ASSERT (Private->BootScript == NULL);
+
   Private->PciIo->Io.Read (
                       Private->PciIo,
                       EfiPciIoWidthUint16,
@@ -722,6 +763,10 @@ ClearScreen (
 {
   UINT32  Color;
 
+  if (Private->BootScript != NULL) {
+    return;
+  }
+
   Color = 0;
   Private->PciIo->Mem.Write (
                         Private->PciIo,
diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c
index 30aac7f..d7179c3 100644
--- a/OvmfPkg/QemuVideoDxe/Gop.c
+++ b/OvmfPkg/QemuVideoDxe/Gop.c
@@ -16,6 +16,7 @@
 #include "Qemu.h"
 #include <IndustryStandard/Acpi.h>
 #include <Library/BltLib.h>
+#include <Pi/PiS3BootScript.h>
 
 STATIC
 VOID
@@ -296,6 +297,30 @@ Returns:
   return Status;
 }
 
+VOID
+Cirrus5446BootScript (
+  QEMU_VIDEO_PRIVATE_DATA *Private,
+  UINT32                  ModeIndex
+  )
+{
+  EFI_S3_SAVE_STATE_PROTOCOL *Script;
+  STATIC CONST CHAR8         Begin[] = "Cirrus 5446 Begin";
+  STATIC CONST CHAR8         End[]   = "Cirrus 5446 End";
+
+  if (EFI_ERROR (gBS->LocateProtocol (&gEfiS3SaveStateProtocolGuid, NULL,
+                        (VOID **) &Script))) {
+    return;
+  }
+
+  Script->Write (Script, EFI_BOOT_SCRIPT_INFORMATION_OPCODE,
+            (UINT32) sizeof Begin, (EFI_PHYSICAL_ADDRESS)(UINTN) Begin);
+  Private->BootScript = Script;
+  InitializeCirrusGraphicsMode (Private, &QemuVideoCirrusModes[ModeIndex]);
+  Private->BootScript = NULL;
+  Script->Write (Script, EFI_BOOT_SCRIPT_INFORMATION_OPCODE,
+            (UINT32) sizeof End, (EFI_PHYSICAL_ADDRESS)(UINTN) End);
+}
+
 EFI_STATUS
 QemuVideoGraphicsOutputConstructor (
   QEMU_VIDEO_PRIVATE_DATA  *Private
@@ -335,6 +360,13 @@ QemuVideoGraphicsOutputConstructor (
   Private->LineBuffer                   = NULL;
 
   //
+  // Prepare boot script for Cirrus
+  //
+  if (Private->Variant == QEMU_VIDEO_CIRRUS_5446) {
+    Cirrus5446BootScript (Private, 1);
+  }
+
+  //
   // Initialize the hardware
   //
   GraphicsOutput->SetMode (GraphicsOutput, 0);
-- 
1.8.3.1

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349831&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to