Re: [Qemu-devel] [PATCH] spice: add qxl device

[Gerd Hoffmann]

  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