On 07/21/2011 12:43 AM, Jan Kiszka wrote:
On 2011-07-20 19:43, Avi Kivity wrote:
> On 07/20/2011 08:41 PM, Jan Kiszka wrote:
>> On 2011-07-20 18:49, Avi Kivity wrote:
>> > New in this version:
>> > - more mindless conversions; I believe there are no longer any
>> destructive
>> > operations in the tree (IO_MEM_UNASSIGNED)
>> > - fix memory map generation bug (patch 13)
>> > - proper 440FX PAM/SMRAM and PCI holes
>> >
>>
>> This on top fixes standard VGA dirty logging:
>
> Both work for me without any patches.
Impossible! ;)
VGA frame buffer cannot work as no one enabled dirty logging for that
range so far. Try -vga std with vga=0x314 in the guest.
Right, actually booting into X showed that. But I don't understand how
it worked before - my patches only change how vga_start_dirty_log() is
implemented, not when/where it is called.
>
> Maybe the F15 window manager is polling the display?
>
Only if that continuously enforces a full window refresh.
Turned out to be a tester issue.
As expected, there were dirty logging issues around removing a
subregion on cirrus bank pointer updates. This makes linear vram
mappings work again:
Please sign off patches!
diff --git a/memory.c b/memory.c
index a8d4295..14fac8a 100644
--- a/memory.c
+++ b/memory.c
@@ -1093,9 +1093,26 @@ void
memory_region_add_subregion_overlap(MemoryRegion *mr,
void memory_region_del_subregion(MemoryRegion *mr,
MemoryRegion *subregion)
{
+ MemoryRegion *target_region;
+ ram_addr_t base, offs;
+
assert(subregion->parent == mr);
subregion->parent = NULL;
QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
+
+ if (subregion->alias) {
+ base = subregion->alias_offset;
+ target_region = subregion->alias;
+ } else {
+ base = 0;
+ target_region = subregion;
+ }
+ if (target_region->dirty_log_mask) {
+ for (offs = 0; offs< subregion->size; offs += TARGET_PAGE_SIZE) {
+ memory_region_set_dirty(target_region, base + offs);
+ }
+ }
+
The subregion may be partially or fully obstructed. This needs to be
done at the FlatRange level (as_memory_range_del(), most likely).
memory_region_update_topology();
}
Debugging provided some more insights:
- address_space_update_topology is not very successful in avoiding
needless mapping updates. kvm_client_set_memory is called much more
frequently than with the old code.
- The region update pattern delete old / add new, e.g. to move the
cirrus bank pointer, will never allow an optimal number of memory
client calls. We either need some memory_region_update or a
transaction API. I would favor the former, should also simplify the
usage.
I had the same thoughts. In-place updates will get more and more
complicated, though. Perhaps we can do a memory_region_replace() (_del
and _add)? That can atomically replace both banks in one go.
Note you can emulate _replace() by adding a new region with a higher
priority, then removing the old one underneath.
- memory_region_update_topology should take a hint if all or just a
specific address space need updating.
It's a nice optimization but I want to defer that to later (when
memory.c actually supersedes ram_addr_t).
- Something makes the startup of graphical grub under cirrus horribly
slow, likely some bug that prevents linear vram mode during the
screen setup. But once it is fully painted for the first time, grub
feels as fast as with the old code.
I haven't seen that. How slow is slow? maybe my eyes aren't fast enough.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
