From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjRFj-0001F8-2W for qemu-devel@nongnu.org; Mon, 03 Jun 2013 05:40:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UjRFa-0005qT-ML for qemu-devel@nongnu.org; Mon, 03 Jun 2013 05:40:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjRFa-0005q1-DX for qemu-devel@nongnu.org; Mon, 03 Jun 2013 05:40:26 -0400 Message-ID: <51AC647B.6090007@redhat.com> Date: Mon, 03 Jun 2013 11:40:11 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1370187812-13191-1-git-send-email-pbonzini@redhat.com> <51AC3C09.2010708@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 00/15] Memory/IOMMU patches part 4: region ownership List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org, Liu Ping Fan Il 03/06/2013 11:22, Peter Maydell ha scritto: >>> Who owns it at that point? [That's a legitimate thing to do, I think, >>> though I don't suppose anybody does it at the moment. >>> Sysbus MMIOs aren't only for mapping in the system address >>> space, they're a general way for one device to expose a >>> MemoryRegion * for use by another device.] >> >> I don't think it is legitimate, MMIO regions are just for use via >> sysbus_map_mmio. > > This is definitely not true. Indeed a wrong generalization. (Though it is becomes almost true if you replace sysbus_map_mmio with memory_region_add_subregion, for which sysbus_map_mmio is a simple wrapper). > We already make extensive use > of MMIO regions other than simply directly via sysbus_map_mmio. > Exposing a MemoryRegion* is just saying "here is something I > have which is some kind of memory mapped IO, do whatever you > need to with it" (which might be mapping it directly to the > system address space, or might be passing it to some other > device that wants a MemoryRegion*, or might be putting it in > a container MR or otherwise managing it). For example, > arm11mpcore.c does this: > sysbus_init_mmio(dev, sysbus_mmio_get_region(s->priv, 0)); > which I suspect will assert with your patches. Thanks for the pointer. All other occurrences of sys_bus_mmio_get_region are either in non-qdevified OMAP code, or they do what I said in my patch. I'll fix a11mpcore to use an alias. >> The right thing to do is to use a container or alias region, and put the >> 1st region inside it. Then the 1st region keeps its owner, and the >> container/alias gets a new one. > > I think the actual right fix is to make the creator of > the MR specify its owner. Anything else is just going to > have holes in it. I think the rules I wrote down are easy to understand, and I'd really like to avoid touching 783 instances of memory_region_init*. The patches say that devices doing their own stuff with regions are the exception, not the rule. Thus the bus functions (which already take a DeviceState) are just as good a place to set ownership as memory_region_init*. Paolo