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 5206D2119359F for ; Tue, 20 Nov 2018 11:59:37 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7F125308A959; Tue, 20 Nov 2018 19:59:36 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-171.rdu2.redhat.com [10.10.120.171]) by smtp.corp.redhat.com (Postfix) with ESMTP id C541D5D6A9; Tue, 20 Nov 2018 19:59:34 +0000 (UTC) From: Laszlo Ersek To: yuchenlin@synology.com Cc: edk2-devel@lists.01.org, phil@philjordan.eu, jordan.l.justen@intel.com, anthony.perard@citrix.com References: <20181107034713.24907-1-yuchenlin@synology.com> Message-ID: Date: Tue, 20 Nov 2018 20:59:28 +0100 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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.41]); Tue, 20 Nov 2018 19:59:36 +0000 (UTC) Subject: Re: [PATCH v3] OvmfPkg: simply use the Bochs interface for 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, 20 Nov 2018 19:59:38 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/07/18 18:06, Laszlo Ersek wrote: > On 11/07/18 04:47, yuchenlin@synology.com wrote: >> From: yuchenlin >> >> BAR | std vga | vmsvga >> --------------------------------- >> 0 | Framebuffer | I/O space >> 1 | Reserved | Framebuffer >> 2 | MMIO | FIFO >> >> - We cannot recognize VMW SVGA as BOCHS because that would confuse the >> IsQxl setting in QemuVideoControllerDriverStart(), >> >> - We cannot recognize VMW SVGA as BOCHS_MMIO because BAR2 on VMW SVGA is >> not the BOCHS MMIO BAR (we can only use port IO). >> >> Therefore the list of reasons for which we should introduce >> QEMU_VIDEO_VMWARE_SVGA should name three reasons: >> >> (1) Get framebuffer from correct PCI BAR >> (2) Prevent using BAR2 for MMIO >> (3) Prevent mis-recognizing VMW SVGA as QXL >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: yuchenlin >> --- >> Changelog: >> >> v1 -> v2 >> * use 'else' clause (Thanks Philippe). >> * add more comment in revert patches (Thanks Philippe). >> * reorder the revert patches, we should revert the last commit first. >> * use correct framebuffer to ClearScreen. >> * revert VMWare svga definitions. >> >> v2 -> v3 >> * Update commit message (Thanks Laszlo) >> * Treat QEMU_VIDEO_VMWARE_SVGA as QEMU_VIDEO_BOCHS (Thanks Laszlo) >> >> --- >> OvmfPkg/QemuVideoDxe/Driver.c | 16 +++++++++++++++- >> OvmfPkg/QemuVideoDxe/Gop.c | 2 +- >> OvmfPkg/QemuVideoDxe/Qemu.h | 2 ++ >> 3 files changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/OvmfPkg/QemuVideoDxe/Driver.c b/OvmfPkg/QemuVideoDxe/Driver.c >> index 2304afd1e6..8e02700d39 100644 >> --- a/OvmfPkg/QemuVideoDxe/Driver.c >> +++ b/OvmfPkg/QemuVideoDxe/Driver.c >> @@ -69,6 +69,12 @@ QEMU_VIDEO_CARD gQemuVideoCardList[] = { >> 0x1050, >> QEMU_VIDEO_BOCHS_MMIO, >> L"QEMU VirtIO VGA" >> + },{ >> + PCI_CLASS_DISPLAY_VGA, >> + 0x15ad, >> + 0x0405, >> + QEMU_VIDEO_VMWARE_SVGA, >> + L"QEMU VMWare SVGA" >> },{ >> 0 /* end of list */ >> } >> @@ -316,6 +322,14 @@ QemuVideoControllerDriverStart ( >> } >> } >> >> + // >> + // VMWare SVGA is handled like Bochs (with port IO only). >> + // >> + if (Private->Variant == QEMU_VIDEO_VMWARE_SVGA) { >> + Private->Variant = QEMU_VIDEO_BOCHS; >> + Private->FrameBufferVramBarIndex = PCI_BAR_IDX1; >> + } >> + >> // >> // Check if accessing the bochs interface works. >> // >> @@ -764,7 +778,7 @@ ClearScreen ( >> Private->PciIo->Mem.Write ( >> Private->PciIo, >> EfiPciIoWidthFillUint32, >> - 0, >> + Private->FrameBufferVramBarIndex, >> 0, >> 0x400000 >> 2, >> &Color >> diff --git a/OvmfPkg/QemuVideoDxe/Gop.c b/OvmfPkg/QemuVideoDxe/Gop.c >> index d490fa7a2e..6f542d9eac 100644 >> --- a/OvmfPkg/QemuVideoDxe/Gop.c >> +++ b/OvmfPkg/QemuVideoDxe/Gop.c >> @@ -60,7 +60,7 @@ QemuVideoCompleteModeData ( >> >> Private->PciIo->GetBarAttributes ( >> Private->PciIo, >> - 0, >> + Private->FrameBufferVramBarIndex, >> NULL, >> (VOID**) &FrameBufDesc >> ); >> diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h >> index d7da761705..3aac9eeca6 100644 >> --- a/OvmfPkg/QemuVideoDxe/Qemu.h >> +++ b/OvmfPkg/QemuVideoDxe/Qemu.h >> @@ -92,6 +92,7 @@ typedef enum { >> QEMU_VIDEO_CIRRUS_5446, >> QEMU_VIDEO_BOCHS, >> QEMU_VIDEO_BOCHS_MMIO, >> + QEMU_VIDEO_VMWARE_SVGA, >> } QEMU_VIDEO_VARIANT; >> >> typedef struct { >> @@ -120,6 +121,7 @@ typedef struct { >> QEMU_VIDEO_VARIANT Variant; >> FRAME_BUFFER_CONFIGURE *FrameBufferBltConfigure; >> UINTN FrameBufferBltConfigureSize; >> + UINT8 FrameBufferVramBarIndex; >> } QEMU_VIDEO_PRIVATE_DATA; >> >> /// >> > > Reviewed-by: Laszlo Ersek > > I'm not pushing this patch at once, for two reasons: > > - I should leave time for other reviewers to comment, > - we are now in the soft feature freeze period > , > and this is not a bugfix, but a feature (re-)enablement that is only now > being reviewed. > > I'll keep this tagged on my queue until after the edk2-stable201811 tag > is pushed, and then I'll push this patch. Please do ping me, should I > forget. I haven't forgotten. :) * First, I've cherry-picked the first four patches (from the v2 series) from the git history. See . * Second, I've added PhilMD's R-b to those four patches. See . * Third, PhilMD got busy with other stuff meanwhile, so I've tested the patch I'm replying to, myself. I have confirmed the following device models continue to work: - Cirrus 5446 - QEMU Standard VGA - QEMU QXL VGA - QEMU VirtIO VGA Therefore, for patch #5 (the present patch), Regression-tested-by: Laszlo Ersek * Fourth, I've also determined that the patch enables OVMF to drive: - QEMU VMWare SVGA therefore, for this patch, Tested-by: Laszlo Ersek * Fifth, the full series has been pushed as commit range 7f3b0bad4bbb..d021868ccf49. Thank you for the contribution, and also for your patience! Laszlo