From patchwork Fri Apr 29 16:52:06 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 93455 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 84CF71007D7 for ; Sat, 30 Apr 2011 02:52:23 +1000 (EST) Received: from localhost ([::1]:44868 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFqvT-00062s-PV for incoming@patchwork.ozlabs.org; Fri, 29 Apr 2011 12:52:19 -0400 Received: from eggs.gnu.org ([140.186.70.92]:44083) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFqvM-00062f-78 for qemu-devel@nongnu.org; Fri, 29 Apr 2011 12:52:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QFqvL-0004pq-8J for qemu-devel@nongnu.org; Fri, 29 Apr 2011 12:52:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57470) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QFqvK-0004md-WB for qemu-devel@nongnu.org; Fri, 29 Apr 2011 12:52:11 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p3TGq8Fh000780 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 29 Apr 2011 12:52:09 -0400 Received: from [10.3.113.60] (ovpn-113-60.phx2.redhat.com [10.3.113.60]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p3TGq7p4023319; Fri, 29 Apr 2011 12:52:08 -0400 From: Alex Williamson To: Jan Kiszka In-Reply-To: <1304091508.3418.11.camel@x201> References: <20110429031437.3796.49456.stgit@s20.home> <20110429150640.GB27816@redhat.com> <4DBAD942.6080001@siemens.com> <1304091508.3418.11.camel@x201> Date: Fri, 29 Apr 2011 10:52:06 -0600 Message-ID: <1304095926.14244.5.camel@x201> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] Fix phys memory client - pass guest physical address not region offset X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Fri, 2011-04-29 at 09:38 -0600, Alex Williamson wrote: > On Fri, 2011-04-29 at 17:29 +0200, Jan Kiszka wrote: > > On 2011-04-29 17:06, Michael S. Tsirkin wrote: > > > On Thu, Apr 28, 2011 at 09:15:23PM -0600, Alex Williamson wrote: > > >> When we're trying to get a newly registered phys memory client updated > > >> with the current page mappings, we end up passing the region offset > > >> (a ram_addr_t) as the start address rather than the actual guest > > >> physical memory address (target_phys_addr_t). If your guest has less > > >> than 3.5G of memory, these are coincidentally the same thing. If > > > > I think this broke even with < 3.5G as phys_offset also encodes the > > memory type while region_offset does not. So everything became RAMthis > > way, no MMIO was announced. > > > > >> there's more, the region offset for the memory above 4G starts over > > >> at 0, so the set_memory client will overwrite it's lower memory entries. > > >> > > >> Instead, keep track of the guest phsyical address as we're walking the > > >> tables and pass that to the set_memory client. > > >> > > >> Signed-off-by: Alex Williamson > > > > > > Acked-by: Michael S. Tsirkin > > > > > > Given all this, can yo tell how much time does > > > it take to hotplug a device with, say, a 40G RAM guest? > > > > Why not collect pages of identical types and report them as one chunk > > once the type changes? > > Good idea, I'll see if I can code that up. I don't have a terribly > large system to test with, but with an 8G guest, it's surprisingly not > very noticeable. For vfio, I intend to only have one memory client, so > adding additional devices won't have to rescan everything. The memory > overhead of keeping the list that the memory client creates is probably > also low enough that it isn't worthwhile to tear it all down if all the > devices are removed. Thanks, Here's a first patch at a patch to do this. For a 4G guest, it reduces the number of registration induced set_memory callbacks from 1048866 to 296. Signed-off-by: Alex Williamson diff --git a/exec.c b/exec.c index e670929..5510b0b 100644 --- a/exec.c +++ b/exec.c @@ -1741,8 +1741,15 @@ static int cpu_notify_migration_log(int enable) return 0; } +struct last_map { + target_phys_addr_t start_addr; + ram_addr_t size; + ram_addr_t phys_offset; +}; + static void phys_page_for_each_1(CPUPhysMemoryClient *client, - int level, void **lp, target_phys_addr_t addr) + int level, void **lp, + target_phys_addr_t addr, struct last_map *map) { int i; @@ -1754,15 +1761,28 @@ static void phys_page_for_each_1(CPUPhysMemoryClient *client, addr <<= L2_BITS + TARGET_PAGE_BITS; for (i = 0; i < L2_SIZE; ++i) { if (pd[i].phys_offset != IO_MEM_UNASSIGNED) { - client->set_memory(client, addr | i << TARGET_PAGE_BITS, - TARGET_PAGE_SIZE, pd[i].phys_offset); + target_phys_addr_t cur = addr | i << TARGET_PAGE_BITS; + if (map->size && + cur == map->start_addr + map->size && + pd[i].phys_offset == map->phys_offset + map->size) { + + map->size += TARGET_PAGE_SIZE; + continue; + } else if (map->size) { + client->set_memory(client, map->start_addr, + map->size, map->phys_offset); + } + + map->start_addr = addr | i << TARGET_PAGE_BITS; + map->size = TARGET_PAGE_SIZE; + map->phys_offset = pd[i].phys_offset; } } } else { void **pp = *lp; for (i = 0; i < L2_SIZE; ++i) { phys_page_for_each_1(client, level - 1, pp + i, - (addr << L2_BITS) | i); + (addr << L2_BITS) | i, map); } } } @@ -1770,9 +1790,15 @@ static void phys_page_for_each_1(CPUPhysMemoryClient *client, static void phys_page_for_each(CPUPhysMemoryClient *client) { int i; + struct last_map map = { 0 }; + for (i = 0; i < P_L1_SIZE; ++i) { phys_page_for_each_1(client, P_L1_SHIFT / L2_BITS - 1, - l1_phys_map + i, i); + l1_phys_map + i, i, &map); + } + if (map.size) { + client->set_memory(client, map.start_addr, + map.size, map.phys_offset); } }