From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [RFC v5 00/86] Memory API Date: Thu, 21 Jul 2011 11:37:01 +0300 Message-ID: <4E27E52D.1090600@redhat.com> References: <1311180636-17012-1-git-send-email-avi@redhat.com> <4E27132E.6080504@siemens.com> <4E2713C9.1030604@redhat.com> <4E274C06.40902@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:16954 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751905Ab1GUIhF (ORCPT ); Thu, 21 Jul 2011 04:37:05 -0400 In-Reply-To: <4E274C06.40902@web.de> Sender: kvm-owner@vger.kernel.org List-ID: 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