From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=35981 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PIhq6-0004FI-LH for qemu-devel@nongnu.org; Wed, 17 Nov 2010 08:14:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PIhq4-0003qH-MP for qemu-devel@nongnu.org; Wed, 17 Nov 2010 08:14:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53896) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PIhq4-0003q6-EC for qemu-devel@nongnu.org; Wed, 17 Nov 2010 08:14:16 -0500 Message-ID: <4CE3D522.4080504@redhat.com> Date: Wed, 17 Nov 2010 14:14:10 +0100 From: Gerd Hoffmann MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] spice: add qxl device References: <1288704898-30234-1-git-send-email-kraxel@redhat.com> <4CE29404.7070008@codemonkey.ws> In-Reply-To: <4CE29404.7070008@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org Hi, > These headers shouldn't be needed. This file also needs a copyright.diff > --git a/hw/qxl-render.c b/hw/qxl-render.c > > Does QXL have a specification? The spice protocol has a specification (slightly outdated though). http://www.spice-space.org/docs/spice_protocol.pdf That covers the rendering commands. That are certainly not all aspects of the qxl device though. >> +static void init_qxl_rom(PCIQXLDevice *d) >> +{ > > You don't really mean ROM, right. This is a set of configuration > registers exposed to the guest via PCI, right? It isn't the option rom bar. It is a memory bar with (read-only) configuration information for the guest. >> + QXLRom *rom = qemu_get_ram_ptr(d->rom_offset); > > This function should not be used in a device model. --verbose please. I think this one I can replace with cpu_physical_memory_write(). There are other cases though where I hand pointers to device memory (commands, image data) to the spice server. cpu_physical_memory_rw() wouldn't work there. >> +device_init(qxl_register); > > I don't see how compatibility works at all with having the QXL device in > a separate library. How do we control what features are enabled/disabled > in the QXL device? There are no features one can enable or disable. What we have are spice server config options which (for example) affect how the spice server sends image data to the client, i.e. whenever lossy jpeg compression (aka wan support) is enabled or not. That isn't visible to the guest and the qxl device though. There are two revisions of the qxl device: - spice 0.4.x is pci revision 1. - spice 0.6.x is pci revision 2 (this submission). The qxl device is backward-compatible, i.e. spice 0.4.x guest drivers can continue to work with the current qxl device. Nevertheless there is a special compat mode (activated via qxl.rev=1 property) which makes the current qxl device look exactly like a spice 0.4.x one. This is needed for two reasons: - migration compatibility. - some spice 0.4.x guest drivers are quite picky and refuse to load if they find a pci revision != 1. It isn't clear yet how we'll handle any guest-visible changes to the spice display protocol such as new rendering commands. One option is to raise the pci revision again. Another option is to add feature flags to the rom bar (the configuration area, not option rom bar). Enabling/disabling these features can be handled using qdev properties. > Is there a specification of the QXL device? See above. > I would like to understand > why we need to emulate the QXL device outside of QEMU. Usually the rendering doesn't happen at all on the server side ... > N.B. I'm *not* advocating implementing Spice in QEMU but AFAICT, QXL > commands can either be rendered locally or offloaded via Spice. Is it > within the realm of possibility to have a local rendering mode entirely > within QEMU and then have an explicit command overload interface that we > can tie into libspice? Usually the rendering commands are send using the spice protocol to the spice client and are rendered there. Server-side local rendering happens in some special cases (read back video memory for screen shots, flush state for migration). Local rendering is also used to handle vnc/sdl. Have qemu parse and process the rendering commands for sdl/vnc is certainly possible. You would effectively have to copy the spice rendering engine into qemu though. It would be alot of work and alot of code which is duplicated for IMHO no good reason. > With QEMU decoding the QXL commands, it means > that QEMU provides the guest visible interface which is useful from an > architectural security PoV. --verbose. cheers, Gerd