From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55465) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tj9oX-0007Yz-2G for qemu-devel@nongnu.org; Thu, 13 Dec 2012 09:31:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tj9oS-0003sQ-KF for qemu-devel@nongnu.org; Thu, 13 Dec 2012 09:31:04 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17307) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tj9oS-0003sM-CN for qemu-devel@nongnu.org; Thu, 13 Dec 2012 09:31:00 -0500 Message-ID: <50C9E697.6040802@redhat.com> Date: Thu, 13 Dec 2012 09:30:47 -0500 From: Yonit Halperin MIME-Version: 1.0 References: <466983004.47092829.1355395217322.JavaMail.root@redhat.com> In-Reply-To: <466983004.47092829.1355395217322.JavaMail.root@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qxl: use correct rom size for revision < 4 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: =?UTF-8?B?U8O4cmVuIFNhbmRtYW5u?= , Uri Lublin , Gerd Hoffmann , Arnon Gilboa , qemu-devel@nongnu.org On 12/13/2012 05:40 AM, Alon Levy wrote: >> On 12/06/12 16:41, Alon Levy wrote: >>> RHBZ 869981 >>> >>> Before this patch revision < 4 (4 is the default) would result in a >>> wrong >>> qxl_rom size of 16384 instead of 8192 when building with >>> spice-protocol-0.12, due to the addition of fields in >>> the rom for client capabilities and monitors config that were added >>> between spice-protocol 0.10 and 0.12. >>> >>> The solution is a bit involved, since I decided not to change >>> QXLRom >>> which is defined externally in spice-protocol. Instead for revision >>> < 4 >>> we allocate 72 bytes for the QXLRom on the qxl_rom bar (bytes >>> [0,71]) >>> and make sure no fields out of that range are accessed, via >>> checking of >>> the revision and nop-ing. >> >> Ok, I see we tackle two issues here. >> >> Number one is qxl accessing the new fields with revision being < 4. >> That needs fixing indeed. But separate patch please. > > Will do. > >> >> Number two is breaking migration due to the rom size change. Can't >> we >> just get the rom below 8k again instead? I think we can throw away a >> whole bunch of modes. Each mode is four times in the list, for >> orientation = { 0, 1, 2, 3 }. orientation is never ever used >> anywhere, >> looks like historic leftover or something planned which was never >> actually implemented. >> >> So keeping orientation = 0 only and kick out everything else should >> give >> us plenty of room ... > > That is indeed a better solution, but it does change functionality. I think it is correct but I'd like to get some other opinions - Uri, Arnon, Yonit, Soren - any problems with dropping these? > Orientation is used in the Windows Display driver. It is used to set dmDisplayOrientation in DEVMODEW structure (http://msdn.microsoft.com/en-us/library/windows/hardware/ff552837(v=vs.85).aspx). So, I'm not sure we can just drop it. Moreover, we need at least 2 of the orientations, one for AxB resolution and the other for BxA. >> >> cheers, >> Gerd >> >> >