From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjSa3-0003zL-V4 for qemu-devel@nongnu.org; Mon, 03 Jun 2013 07:05:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UjSZz-0001dD-Be for qemu-devel@nongnu.org; Mon, 03 Jun 2013 07:05:39 -0400 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:64068) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UjSZz-0001d7-4T for qemu-devel@nongnu.org; Mon, 03 Jun 2013 07:05:35 -0400 Received: by mail-wg0-f51.google.com with SMTP id b13so3079940wgh.30 for ; Mon, 03 Jun 2013 04:05:34 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <51AC7871.9060500@redhat.com> Date: Mon, 03 Jun 2013 13:05:21 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1370187812-13191-1-git-send-email-pbonzini@redhat.com> <51AC3C09.2010708@redhat.com> <51AC647B.6090007@redhat.com> <51AC6BF6.9080908@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 12:25, Peter Maydell ha scritto: > On 3 June 2013 11:12, Paolo Bonzini wrote: >> 1) I could set the owner to NULL before calling the sysbus_init_mmio; >> >> 2) I could add a variant of sysbus_init_mmio that doesn't set the owner; >> >> 3) I could skip setting the owner for sysbus altogether, since it is >> only strictly required for unpluggable devices. > > 4) you could set the owner at the right place, ie when the > memory region is created. > >> However, I think there is worth in preserving the chain through either >> containment or aliasing. From the nesting point of view, >> realview_mpcore is exposing the region. From the implementor point of >> view, arm_gic is implementing the region (and arm_gic is the one that >> would be ref/unref'd at execution time). In either case, >> arm11mpcore_priv is not what you want to see. Its presence is just an >> implementation detail of realview_mpcore. > > I agree that the owner of the MR in this case should be > arm_gic. The owner of the I/O region is arm_gic. It is set when arm_gic does sysbus_init_mmio. But container regions have an owner too. Here the owner is set initially to arm11mpcore_priv, and then it bombs when realview_mpcore tries to change it to itself. Make each level wrap the region with a container makes sense to me. But actually sysbus_pass_mmio would match nicely sysbus_pass_irq as an API, so (2) above would be a good choice too. What I'm definitely not going to do, is go through 800 calls and add owners to all of them. > For aliasing, surely you need to actually reference > both devices? If the device that owns the container goes > away then your MR* is toast anyway, and if the device doing > the actual implementation goes away your MR* is also now > invalid. So they have to both hang around long enough for > you to finish whatever you were doing. Container and alias regions are "virtual", they are eliminated while creating the flat memory map; I/O goes straight to the contained/aliased region. Hence, containers and aliases in principle need no owner; it is still nice to have one, for symmetry and ease of debugging. A reference to the aliased region is added when: (1) the region is added to the flat memory map; (2) the alias is created. Both are done in patch 3. Paolo