From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41234) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T3nmi-00062h-DP for qemu-devel@nongnu.org; Tue, 21 Aug 2012 08:42:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T3nmc-000489-H8 for qemu-devel@nongnu.org; Tue, 21 Aug 2012 08:42:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34251) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T3nmc-000480-8O for qemu-devel@nongnu.org; Tue, 21 Aug 2012 08:42:10 -0400 Message-ID: <503381EE.9060003@redhat.com> Date: Tue, 21 Aug 2012 15:41:18 +0300 From: Avi Kivity MIME-Version: 1.0 References: <502A2D26.4060702@redhat.com> <502CBC26.9030705@redhat.com> <5030BC1D.90801@redhat.com> <5030CFB1.7010704@redhat.com> <50334D78.7020004@redhat.com> <50335F50.9080104@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] memory: could we add extra input param for memory_region_init_io()? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: liu ping fan Cc: Peter Maydell , qemu-devel@nongnu.org, Anthony Liguori On 08/21/2012 02:18 PM, liu ping fan wrote: >> >>> But as >>> it will also take the code path which has object_ref(Object*), so it >>> has to convert, otherwise the code will corrupt. >>> That is what I want to express. >> >> Option 2, for example, had MemoryRegionOps::object(MemoryRegion *) which >> can be used to convert the opaque to an Object, or return NULL. With >> that option, there would be no corruption: >> > If we did not pass extra flag to indicate whether opaque is Object or > not, we can not implement MemoryRegionOps::object(). Because we lack > of something like "try,catch", and object_dynamic_cast() can not work. > So besides memory_region_init_io(), we need extra interface to pass that flag? The interface is MemoryRegionOps::object(). If the user does not implement it, then the opaque cannot be converted into an object. If the user did implement it, then the function specifies how to convert it. For example: static Object *e1000_mmio_object(void *opaque) { E1000State *s = opaque; return &s->dev.qdev.parent_obj; } static const MemoryRegionOps e1000_mmio_ops = { .read = e1000_mmio_read, .write = e1000_mmio_write, .endianness = DEVICE_LITTLE_ENDIAN, .object = e1000_mmio_object, .impl = { .min_access_size = 4, .max_access_size = 4, }, }; >> qemu_mutex_lock(&mem_map_lock); >> MemoryRegionSection mrs = walk(&phys_map); >> Object *object = mrs.mr->ops->object(mrs.mr); >> if (object) { >> object_ref(object); >> } >> qemu_mutex_unlock(&mem_map_unlock); >> This should read: Object *object = NULL; if (mrs.mr->ops->object) { object = mrs.mr->ops->object(mrs.mr); } >> So there is no memory corruption. However, as I point out below, we >> also lack the reference count. Maybe we could do something like: >> > I think the hotplug issue is only for limited type of bus. And > fortunately, for pci, they have already adopt Object. > So for other SoC, maybe we can ignore this issue at current step We still have to take the big qemu lock somehow. >> If we can avoid a cycle. Won't the device need to hold refs to the BAR? > > I will check the code, but I has thought BAR is implemented by > assigned device? Yes. -- error compiling committee.c: too many arguments to function