Re: [RFC v5 00/86] Memory API

Jan Kiszka
2011-07-20T21:43:39Z

[-- Attachment #1: Type: text/plain, Size: 2773 bytes --]

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.

>
> Maybe the F15 window manager is polling the display?
>

Only if that continuously enforces a full window refresh.

As expected, there were dirty logging issues around removing a
subregion on cirrus bank pointer updates. This makes linear vram
mappings work again:

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);
+ }
+ }
+
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.
- memory_region_update_topology should take a hint if all or just a
specific address space need updating.
- 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.

Jan

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]