From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60302 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OCCuE-0003q0-Ox for qemu-devel@nongnu.org; Wed, 12 May 2010 10:27:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OCCuC-0008PQ-40 for qemu-devel@nongnu.org; Wed, 12 May 2010 10:27:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7884) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OCCuB-0008OX-Ga for qemu-devel@nongnu.org; Wed, 12 May 2010 10:27:23 -0400 Message-ID: <4BEABABC.6080305@redhat.com> Date: Wed, 12 May 2010 17:27:08 +0300 From: Avi Kivity MIME-Version: 1.0 References: <4BE32178.2090103@msgid.tls.msk.ru> <4BE7B8C1.9060807@redhat.com> <4BE7C0A5.3090909@redhat.com> <4BEAA0CC.4090906@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: Another SIGFPE in display code, now in cirrus List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: Brian Kress , Michael Tokarev , qemu-devel , KVM list On 05/12/2010 04:45 PM, Stefano Stabellini wrote: > > >> Note it's just during mode changes. During normal operation I'm sure >> the pitches are equal. >> >> > The source blt pitch as set by the driver is always equal to the display > pitch (apart from the case reported above). > However cirrus_blt_srcpitch is not always equal to the display pitch > because of CIRRUS_BLTMODE_BACKWARDS: cirrus_blt_srcpitch can also be > negative and equal to -display_pitch. > Yes. > I suggest to start using the display pitch (with the proper sign) > instead of cirrus_blt_srcpitch in cirrus_do_copy at least when > cirrus_blt_srcpitch doesn't have a proper value. > Why switch from one bug to the other? It's perfectly possible to take into account both values. Of course when abs(blt_pitch) != display pitch we can't use console_copy, but so what. >> 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 >> >> > I think the following should be enough: > > if bitblt destination intersects display memory: > qemu_console_copy > else > invalidate region > > why do we need if bitblt pitch == display pitch or to invalidate > everything? > Because the region is not a rectangle anymore. We could compute exactly what needs to be invalidated, but since it will never happen, there's no point in optimizing it. >>>> 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. >> >> > Sure, we have to fix the other one first :) > And find it. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.