From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 092E8211799E3 for ; Tue, 23 Oct 2018 05:46:07 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3D2413082E07; Tue, 23 Oct 2018 12:46:06 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-214.rdu2.redhat.com [10.10.120.214]) by smtp.corp.redhat.com (Postfix) with ESMTP id 90ED276503; Tue, 23 Oct 2018 12:46:00 +0000 (UTC) To: yuchenlin Cc: edk2-devel@lists.01.org, jordan.l.justen@intel.com, ard.biesheuvel@linaro.org, anthony.perard@citrix.com, julien.grall@linaro.org, Gerd Hoffmann , Phil Dennis-Jordan References: <20181023024057.21942-1-yuchenlin@synology.com> <3f1bacc3feaf1ce6de3b42f91cf689ea@synology.com> From: Laszlo Ersek Message-ID: <917454c5-3420-aba3-097f-0ac46b6792c4@redhat.com> Date: Tue, 23 Oct 2018 14:45:59 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <3f1bacc3feaf1ce6de3b42f91cf689ea@synology.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Tue, 23 Oct 2018 12:46:06 +0000 (UTC) Subject: Re: [PATCH] OvmfPkg: initialize bochs when initializing vmsvga X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Oct 2018 12:46:08 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 >>> >>> 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