[Spice-devel] qxl-win important issues

[Yonit Halperin]

Hi,
In the last days I encountered several BSODs in qxl-win, and I started 
investigating them. I have reached some conclusions, and learnt some new 
stuff that I think is important if you wish to make changes in the driver.

The first BSOD resulted when I worked with a dual monitor vm, and 
diabled and enabled the second monitor and changed its resolution at the 
same time. see RHBZ #722954 
(https://bugzilla.redhat.com/show_bug.cgi?id=722954). I attached a patch 
for this bug, and it includes a detailed explanation

The second BSOD (or sometimes a crash) is described in RHBZ #731644 
(https://bugzilla.redhat.com/show_bug.cgi?id=731644).
The key for this bug is understanding that while the miniport is loaded 
to the kernel system memory, the display driver is loades into session 
space. from http://www.codeproject.com/KB/system/driverdev6asp.aspx:

"Session space is the kernel equivalent of process isolation. In user 
mode processes have their own virtual memory address space and in the 
kernel sessions have their own virtual memory address space. System 
space is the kernel memory which is global to all sessions.

A session is an instance of a logged on user which contains its own 
Window Manager, Desktop(s), shell and applications. This is most notable 
in Windows XP "Fast User Switching" in which you can log multiple users 
onto a single machine. Each user is actually in a unique session with a 
unique range of kernel memory known as session space."

This means that the display driver can be loaded several time 
simultaneously, and that global variables are not shared between 
different sessions (!!!important!!!).

The problem is, qxl does use globals :(
Here is how the BSOD/crash occurred:
In rev2, objects (surfaces and other devram objects) stayed alive in the 
pci ram after DrvAssertMode(Disable) was called. Then, when another 
session started, the driver had newly initiated mspace, but an old 
release ring (with objects from the older session's mspace) => crash

In rev3 and rev2: sometimes DrvDisableDriver was called for the older 
driver session, and it performed Reset to all devices, while the other 
session has already started and was valid. Reset shouldn’t be performed 
there (actually, this code was added in order to solve the multiple 
session problem, but worked only with one monitor).

Assuming (need to check) that DrvAssertMode(disable) is called before 
moving to another session, I think that if we make sure we clear all 
memory also for rev2, this crash won't happen. We should also move the 
surfaces_info data structure to pdev (and also the caches).
The other problem left is all the other globals we use, mostly 
semaphores which according to 
http://msdn.microsoft.com/en-us/library/ff568281%28v=vs.85%29.aspx, we 
definitely don't implement correctly, and we also need to reconsider if 
all of them are really needed.

Cheers,
Yonit.