| Message ID | 1336562766-25682-1-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/09/12 13:26, Alon Levy wrote: > The guest drivers should already know how to use async for revision 3. > But since it's still possible to have an older driver with revision 3 > that doesn't check for the revision, require a new parameter > "force_async", which we can later turn to 1 by default. Hmm. PCI revisions are supposed to be backward compatible, I don't think this is a good idea. We might want to log a warning or inform via 'info spice' in case we find a guest using the old, sync commands, so management can figure and warn, but I wouldn't do more. We can think about defining a qxl2 pci device, with a different pci id, where we can throw away old stuff. For such a new & incompatible device I'd like to change a few more things while being at it ... (1) go for a single 32bit mmio register set similar to real hardware. That should replace the rom bar (device info), the fields in the ram area (where interrupt mask etc live) and the io command bar. (2) consider virtio for the rings. Not sure this is worth it as that wouldn't make qxl bus-agnostic, for vga compatibility and device memory we'll still have a dependency on pci ... (3) renumber the remaining bars so we can make the vram bar 64bit, without a 32bit compatibility alias needed. (4) Drop QXL_IO_SET_MODE support and the mode list. Possibly more ... cheers, Gerd
On Wed, May 09, 2012 at 02:51:01PM +0200, Gerd Hoffmann wrote: > On 05/09/12 13:26, Alon Levy wrote: > > The guest drivers should already know how to use async for revision 3. > > But since it's still possible to have an older driver with revision 3 > > that doesn't check for the revision, require a new parameter > > "force_async", which we can later turn to 1 by default. > > Hmm. PCI revisions are supposed to be backward compatible, I don't > think this is a good idea. We might want to log a warning or inform via > 'info spice' in case we find a guest using the old, sync commands, so > management can figure and warn, but I wouldn't do more. > > We can think about defining a qxl2 pci device, with a different pci id, > where we can throw away old stuff. For such a new & incompatible device > I'd like to change a few more things while being at it ... > > (1) go for a single 32bit mmio register set similar to real hardware. > That should replace the rom bar (device info), the fields in the ram > area (where interrupt mask etc live) and the io command bar. > > (2) consider virtio for the rings. Not sure this is worth it as that > wouldn't make qxl bus-agnostic, for vga compatibility and device memory > we'll still have a dependency on pci ... > > (3) renumber the remaining bars so we can make the vram bar 64bit, > without a 32bit compatibility alias needed. > > (4) Drop QXL_IO_SET_MODE support and the mode list. > > Possibly more ... Sounds like a good idea. I'll try to do a patch with the sync warning first - the reason for this patch was trying to prevent deadlocks with boxes, the management in question. The virtio-rings - I know someone is already working on this and is pretty well advanced. Actually I think they are doing a whole new device, but I haven't seen any code yet. Probably not pci. But just replacing the rings should not be that hard (tm) :) > > cheers, > Gerd
diff --git a/hw/qxl.c b/hw/qxl.c index 6c11e70..451e8e5 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1316,6 +1316,23 @@ static void ioport_write(void *opaque, target_phys_addr_t addr, return; } + if (d->force_async && d->revision >= 3) { + switch (io_port) { + case QXL_IO_UPDATE_AREA: + case QXL_IO_MEMSLOT_ADD: + case QXL_IO_CREATE_PRIMARY: + case QXL_IO_DESTROY_PRIMARY: + case QXL_IO_DESTROY_SURFACE_WAIT: + case QXL_IO_DESTROY_ALL_SURFACES: + qxl_guest_bug(d, "sync io %d not supported for revision >= 3", + io_port); + return; + break; + default: + break; + } + } + /* we change the io_port to avoid ifdeffery in the main switch */ orig_io_port = io_port; switch (io_port) { @@ -2036,6 +2053,7 @@ static Property qxl_properties[] = { DEFINE_PROP_UINT32("ram_size_mb", PCIQXLDevice, ram_size_mb, -1), DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram32_size_mb, -1), DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, -1), + DEFINE_PROP_UINT32("force_async", PCIQXLDevice, force_async, 0), DEFINE_PROP_END_OF_LIST(), };
The guest drivers should already know how to use async for revision 3. But since it's still possible to have an older driver with revision 3 that doesn't check for the revision, require a new parameter "force_async", which we can later turn to 1 by default. Signed-off-by: Alon Levy <alevy@redhat.com> --- hw/qxl.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)