[Spice-devel] [xf86 qxl driver PATCH 5/5] qxl_driver: monitors_config: adjust to memory-remap

[Yonit Halperin]

On 01/21/2013 09:28 AM, Uri Lublin wrote:
> On 01/21/2013 04:16 PM, Yonit Halperin wrote:
>> On 01/17/2013 09:26 AM, Uri Lublin wrote:
>>> Resolves: rhbz#883578
>>>
>>> Call qxl_allocate_monitors_config upon memory-remap such
>>> that qxl->monitors_config points to the start of
>>> monitors_config segment in qxl RAM memory.
>>>
>>> Currently after memory remap, it's possible that monitors_config
>>> memory and video-memory (or graphics) overlap, which means
>>> that one may overwrite another.
>>> Specifically in the bug above, monitors_config value are being
>>> overwritten by video pages, and on the destination bad values
>>> are read which cause problems on the server and client.
>>>
>> Can you please explain the path that leads to this overwriting?
>> I see that in qxl_map_memory qxl_allocate_monitors_config is already
>> called.
>>> It may be a good idea to add some protection on the server side,
>>> e.g. calcluate checksum, compare values against modes, or limit
>>> ->count and ->max_allowed and ignore bad monitors_config values
>>>
>>> Also do not memset-0 monitors-config upon allocation (remapping)
>>> to not overwrite likely good configuration (in case it is
>>> being read by the host, e.g. upon migration).
>> I'm not sure that the code in qemu-qxl should even re-read the
>> monitors configuration during pre-save because it was already updated
>> on the UPDATE_MONITORS_CONFIG io call.
>
> Hi Yonit,
>
> The source host does not re-reads the monitors configuration during
> pre-save.
> The destination host reads it during post-reload.
>
> The monitors_config area is migrated together with all the VM memory.
> And the address to the monitors configuration is transferred as state.
>
Yes, you are right, I forgot the monitor_config we save is the address 
and not the monitor configuration itself.
> Thanks,
>      Uri.
>