On 05/12/2010 03:20 PM, Stefano Stabellini wrote:
On Mon, 10 May 2010, Avi Kivity wrote:
On 05/10/2010 10:41 AM, Avi Kivity wrote:
On 05/06/2010 11:07 PM, Michael Tokarev wrote:
There was a bug recently fixed in vnc code.  Apparently
there's something similar in the cirrus emulation as well.
Here it triggers _always_ (including old versions of kvm)
when running windows NT and hitting "test" button in its
display resolution dialog.  Here's what gdb is to say:

Program received signal SIGFPE, Arithmetic exception.
[Switching to Thread 0xf76cab70 (LWP 580)]
0x080c5e45 in cirrus_do_copy (s=0x86134dc, dst=960000, src=0, w=2, h=9)
     at hw/cirrus_vga.c:687
687        sx = (src % ABS(s->cirrus_blt_srcpitch)) / depth;
(gdb) p depth
$1 = 2
(gdb) p s->cirrus_blt_srcpitch
$2 = 0


This qemu-kvm-0.12.3 - actually a debian package of it,
but there's no patches relevant to video applied.

Anything can be done with it?
Well, it's trivial to check for the condition, but how to handle it?

That code is wrong.  It shouldn't be using the bitblt source pitch, but
the actual display pitch.  If the two are different, then the blt
doesn't actually affect a rectangular region, but instead a parallelogram.

Reading some documention about the Cirrus card we are emulating, it
seems that the source pitch is ignored in video to video operations, so
I think you are right here.

From what I've read I don't think it's ignored. Rather there are two separate pitches, one for bitblt and one for display.

I think the code should be something like

    if bitblt destination intersects display memory:
        if bitblt pitch == display pitch
           use console_copy
       else
           invalidate entire display

The Windows NT driver probably relies on this behaviour and doesn't set
the source pitch properly before the bitblt.

Note it's just during mode changes. During normal operation I'm sure the pitches are equal.

31c05501c says this breaks bitblt, but I can't see why this is true.
The copy should update the display.  This is probably due to a
miscalculation of the affected region, and now we have two invalidates
instead of one, reducing performance.

I agree with you: qemu_console_copy does imply that the copied portion
of the screen changed, so there is no reason to invalidate it again if
qemu_console_copy is called.

Well, we can't just revert 31c05501c. There's probably another bug involved.

--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.


Reply via email to