From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=56879 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OCET1-0005VE-TR for qemu-devel@nongnu.org; Wed, 12 May 2010 12:07:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OCESx-0007D3-8Z for qemu-devel@nongnu.org; Wed, 12 May 2010 12:07:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:6260) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OCESw-0007Cx-UJ for qemu-devel@nongnu.org; Wed, 12 May 2010 12:07:23 -0400 Message-ID: <4BEAD232.2040700@redhat.com> Date: Wed, 12 May 2010 19:07:14 +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> <4BEABABC.6080305@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 06:57 PM, Stefano Stabellini wrote: > On Wed, 12 May 2010, Avi Kivity wrote: > >>> 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 see: you want to support the case when abs(src_blt_pitch) != display_pitch, > while I though it is some sort of error. > When blting withinh the screen, it is an error from the point of view of writing a sane driver. My guess is that it's a bug in the NT driver. It's not an error from the point of view of the hardware, or when blting offscreen bitmaps (which can have different geomeries than the display). > Considering that src_blt_pitch can even be 0 (as in the bug report), we > would still need to calculate sx and sy using display_pitch instead, so > the only place in which it would make a difference is when src_blt_pitch > is passed as an argument to cirrus_rop. > cirrus_rop() and its arguments don't need to change. They're correctly using the blt pitch. My point is: x[xy] and d[xy] are only valid if both source and destination overlap the display, and if both src_pitch and dst_pitch are absequal to the display pitch. When they aren't valid, there's no point in calculating them or using them in anything. The raster operation is still valid though. > I guess even a src blt pitch of 0 could be useful there, however in > practice I think the only rop function that was written with this case in > mind has: > > dstpitch -= bltwidth; > srcpitch -= bltwidth; > > if (dstpitch< 0 || srcpitch< 0) { > /* is 0 valid? srcpitch == 0 could be useful */ > return; > } > > at the beginning and the others probably just don't deal with the case > with possibly buggy consequences. > Also the documentation I have states: > > 3CEh index 26h W(R/W): BLT Source Pitch (5426 +) > bit 0-11 (5426-28) Number of bytes in a scanline at the source. > 0-12 (5429 +) do > > if the source BLT is supposed to be the number of bytes in a scanline at > the source, then 0 is not a correct value for it. > It's useful if you have a one-line horizontal pattern you want to propagate all over. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.