From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9Dk7-0003vr-SN for qemu-devel@nongnu.org; Wed, 05 Sep 2012 07:26:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9Dk2-0003Ai-0O for qemu-devel@nongnu.org; Wed, 05 Sep 2012 07:25:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20066) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9Dk1-0003AR-O8 for qemu-devel@nongnu.org; Wed, 05 Sep 2012 07:25:53 -0400 Message-ID: <504736B8.1080706@redhat.com> Date: Wed, 05 Sep 2012 14:25:44 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1345801763-24227-1-git-send-email-qemulist@gmail.com> <503BBED4.9050705@siemens.com> <503BC1EE.4060608@redhat.com> <503BCCA1.10403@siemens.com> <503C9275.5040105@siemens.com> <503E4FF4.7060506@redhat.com> <503E51BB.9070308@siemens.com> <503E5422.5050805@redhat.com> <503F1172.70103@siemens.com> <5041CB73.4050800@redhat.com> <50446FD9.3040805@redhat.com> <504720F8.9020104@redhat.com> <50472B3B.4090401@siemens.com> <50472F23.2080702@redhat.com> <50473375.8050504@siemens.com> In-Reply-To: <50473375.8050504@siemens.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Paolo Bonzini , Liu Ping Fan , liu ping fan , Anthony Liguori , "qemu-devel@nongnu.org" On 09/05/2012 02:11 PM, Jan Kiszka wrote: > On 2012-09-05 12:53, Avi Kivity wrote: >> On 09/05/2012 01:36 PM, Jan Kiszka wrote: >>>> >>>> My current preference is MemoryRegionOps::ref(MemoryRegion *mr) (and a >>>> corresponding unref), which has the following requirements: >>>> >>>> - if the refcount is nonzero, MemoryRegion::opaque is safe to use >>>> - if the refcount is nonzero, the MemoryRegion itself is stable. >>> >>> The second point means that the memory subsystem will cache the region >>> state and apply changes only after leaving a handler that performed them? >> >> No. I/O callbacks may be called after a region has been disabled. >> >> I guess we can restrict this to converted regions, so we don't introduce >> regressions. > > s/can/have to/. This property change will require some special care, > hopefully mostly at the memory layer. A simple scenario from recent patches: > > if () { > memory_region_set_address(&s->pm_io, pm_io_base); > memory_region_set_enabled(&s->pm_io, true); > } else { > memory_region_set_enabled(&s->pm_io, false); > } I am unable to avoid pointing out that this can be collapsed to memory_region_set_address(&s->pm_io, pm_io_base); memory_region_set_enabled(&s->pm_io, ); as the address is meaningless when disabled. Sorry. > > We will have to ensure that set_enabled(..., true) will never cause a > dispatch using an outdated base address. No, this is entirely safe. If the guest changes a region offset concurrently with issuing mmio on it, then it must expect either the old or new offset to be used during dispatch. In either case, the correct intra-region offset will be provided to the I/O callback (no volatile MemoryRegion fields except ->readable (IIRC) are used during dispatch - the rest are all copied into data structures used during dispatch (this is part of what makes the whole thing so rcu friendly). > I think discussing semantics and usage patterns of the new memory API - > outside of the BQL - will be the next big topic. ;) I hope it won't prove to be that complicated. -- error compiling committee.c: too many arguments to function