Re: [Qemu-devel] [PATCH] qxl: use correct rom size for revision < 4

[Yonit Halperin <yhalperi@redhat.com>]

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
>>
>>
>