Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga

[Laszlo Ersek <lersek@redhat.com>]

On 10/23/18 13:12, yuchenlin wrote:
> Hi, Laszlo
>
> On 2018-10-23 18:18, Laszlo Ersek wrote:
>> (1) Adding Gerd (because he maintains video in QEMU), and Phil
>> Dennis-Jordan (for authoring commit c137d9508169,
>> "OvmfPkg/QemuVideoDxe: VMWare SVGA device support", 2017-04-07).
>>
>>
>> On 10/23/18 04:40, yuchenlin@synology.com wrote:
>>> From: yuchenlin <yuchenlin@synology.com>
>>>
>>> When driver doesn't set fifo config, the vmsvga will fall back
>>> to std vga. However, we don't initialize vbe related port. It
>>> causes blank screen in qemu console.
>>
>> (2) The words "when driver doesn't set fifo config" tell me nothing.
>> The QemuVideoDxe directory has zero instances of the word "fifo". The
>> same applies to "OvmfPkg/Include/IndustryStandard/VmwareSvga.h".
>>
>> In addition, the "vmsvga will fall back to std vga" statement is also
>> unclear. Is that a statement about the QEMU device model's behavior?
>>
>
> About FIFO, you can refer to
> https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L34,
>
>
> "The main advantage of VMware's SVGA device over alternatives like VBE
> is that the virtual machine can explicitly indicate which ares of the
> screen have changed by sending update rectangles through the device's
> command FIFO."
>
> According to
> https://github.com/prepare/vmware-svga/blob/master/doc/svga_interface.txt#L533,
>
> to use vmsvga's FIFO. Driver should setup FIFO and set
> SVGA_REG_CONFIG_DONE to 1. However, OVMF doesn't do it. In this case,
> vmsvga will fall back to vga. It is QEMU device model's behavior after
> commit 104bd1dc70 (vmsvga: fix vmsvga_update_display).
>
>> I vaguely suspect that your intent might be to say, "QemuVideoDxe
>> does not perform a necessary configuration step, and therefore it
>> cannot drive QEMU's VMW SVGA device". However, if that is indeed your
>> intent, then I believe something must have changed recently in QEMU,
>> because QemuVideoDxe *definitely* worked when Phil contributed the
>> VMW SVGA driver logic, in commit c137d9508169.
>>
>> Are we talking about a QEMU regression, or a driver-side
>> configuration step that QemuVideoDxe has always missed (and we're
>> being punished for it only now)?
>>
>
> This is QEMU behavior change in commit 104bd1dc70 (vmsvga: fix
> vmsvga_update_display). In my opinion, it is a correct change.
>
>>
>>> This patch will fix "Guest has not initialized the display (yet)"
>>> when using qemu -device vmware-svga with ovmf.
>>
>> Right, as I write above, this definitely worked earlier. I suggest
>> bisecting QEMU (and/or testing older QEMU machine types) to identify
>> the QEMU side change. Once we know that, we can decide whether this
>> is a QEMU regression, or just exposing a long-standing OVMF bug.
>
> In my opinion, it is a long-standing OVMF bug, which is based on the
> wrong behavior of QEMU's vmsvga. It relied on dirty memory region to
> call dpy_gfx_update for updating display.

Thank you for the explanation!

Please help me see the situation better. Here's my current
understanding.

(1) QemuVideoDxe doesn't set up the VMW SVGA FIFO, and does not store 1
    to the SVGA_REG_CONFIG_DONE register. And this is not a "small
    missing step" -- the FIFO setup can be considered a separate
    feature.

(2) We don't intend to implement the FIFO setup feature. (In particular
    because we don't intend to track changed rectangles and send updates
    via the FIFO.)

(3) The intent of the original VMW SVGA enablement patch for
    QemuVideoDxe, namely commit c137d9508169, was to enable booting some
    UEFI operating systems on OVMF that had guest drivers only for VMW
    SVGA.

(4) The QEMU device model now -- since commit 104bd1dc70 -- falls back
    to stdvga (that is, Bochs) in response to QemuVideoDxe's actions.

(5) Your proposal is to set up the Bochs interface in QemuVideoDxe, *in
    addition* to the -- now dysfunctional! -- VMW SVGA interface.

Is my understanding correct?

Because, here's what I think: the VMW SVGA guest driver logic in
QemuVideoDxe is basically dead code now, since QEMU commit 104bd1dc70.
As I understand it, we don't rely on it *at all*. It does nothing for
us.

(And that has been the case since QEMU v2.10.0, released on 2017-Aug-30.
It's quite telling that the first issue report comes in now, after more
than one year...)

So, what do you think of the following approach, instead of your
currently proposed patch:

- revert commit c137d9508169 ("OvmfPkg/QemuVideoDxe: VMWare SVGA device
  support", 2017-04-07)

- revert commit 05a537945872 ("OvmfPkg/QemuVideoDxe: Helper functions
  for unaligned port I/O.", 2017-04-07)

- given that QEMU provides the Bochs interface anyway, with the VMW SVGA
  device model, simply recognize the "QEMU VMWare SVGA" card, in the
  "gQemuVideoCardList" array, as the QEMU_VIDEO_BOCHS_MMIO variant.

Would that work? Because, if our current VMW SVGA driver code is
basically dead, I prefer a solution that simplifies QemuVideoDxe,
instead of complicating it further. And, importantly, guest OS drivers
could *still* set up the real VMW SVGA FIFO thing, after they boot, on
the same device.

The remaining question is how this approach would work with QEMU
versions and/or machine types that precede v2.10.0. To your knowledge:

Before QEMU commit 104bd1dc70, was it really *required* to use the (now
dead) QemuVideoDxe code for VMW SVGA, or would it always have been
possible to simply use the Bochs interface, to drive the VMW SVGA
device?

(I don't think we have ever asked this question. Checking the original
RFC thread:

  [edk2] [RFC PATCH v1 0/2]
  OvmfPkg/QemuVideoDxe: Add VMWare SVGA2 framebuffer support

  https://lists.01.org/pipermail/edk2-devel/2017-March/009201.html

I don't see a question whether using the Bochs VBE interface would
simply work. I think we all assumed that the VMW SVGA-specific interface
was a hard requirement, for enabling QemuVideoDxe on QEMU's VMW SVGA
device model.)


Either way, I would certainly like to hear Gerd's opinion on this.

Thanks!
Laszlo