From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [RFC PATCH v3 05/19] Implement dimm device
	abstraction
Date: Wed, 31 Oct 2012 13:15:20 +0200
Message-ID: <50910848.7000002@redhat.com>
References: <1348226255-4226-1-git-send-email-vasilis.liaskovitis@profitbricks.com>
	<1348226255-4226-6-git-send-email-vasilis.liaskovitis@profitbricks.com>
	<20121023122532.GE19977@stefanha-thinkpad.redhat.com>
	<CAJnKYQk7k=AFpN1LwzDpuOYZcLmYNAaG3BPCBmNDDa+brFWJJQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: blauwirbel@gmail.com, kvm@vger.kernel.org, gleb@redhat.com,
	Stefan Hajnoczi <stefanha@gmail.com>, seabios@seabios.org,
	qemu-devel@nongnu.org,
	Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>,
	kevin@koconnor.net, kraxel@redhat.com, anthony@codemonkey.ws,
	imammedo@redhat.com, Paolo Bonzini <pbonzini@redhat.com>
To: liu ping fan <qemulist@gmail.com>
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
In-Reply-To: <CAJnKYQk7k=AFpN1LwzDpuOYZcLmYNAaG3BPCBmNDDa+brFWJJQ@mail.gmail.com>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
List-Id: kvm.vger.kernel.org

On 10/24/2012 10:06 AM, liu ping fan wrote:
> On Tue, Oct 23, 2012 at 8:25 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Fri, Sep 21, 2012 at 01:17:21PM +0200, Vasilis Liaskovitis wrote:
>>> +static void dimm_populate(DimmDevice *s)
>>> +{
>>> +    DeviceState *dev= (DeviceState*)s;
>>> +    MemoryRegion *new = NULL;
>>> +
>>> +    new = g_malloc(sizeof(MemoryRegion));
>>> +    memory_region_init_ram(new, dev->id, s->size);
>>> +    vmstate_register_ram_global(new);
>>> +    memory_region_add_subregion(get_system_memory(), s->start, new);
>>> +    s->mr = new;
>>> +}
>>> +
>>> +static void dimm_depopulate(DimmDevice *s)
>>> +{
>>> +    assert(s);
>>> +    vmstate_unregister_ram(s->mr, NULL);
>>> +    memory_region_del_subregion(get_system_memory(), s->mr);
>>> +    memory_region_destroy(s->mr);
>>> +    s->mr = NULL;
>>> +}
>>
>> How is dimm hot unplug protected against callers who currently have RAM
>> mapped (from cpu_physical_memory_map())?
>>
>> Emulated devices call cpu_physical_memory_map() directly or indirectly
>> through DMA emulation code.  The RAM pointer may be held for arbitrary
>> lengths of time, across main loop iterations, etc.
>>
>> It's not clear to me that it is safe to unplug a DIMM that has network
>> or disk I/O buffers, for example.  We also need to be robust against
>> malicious guests who abuse the hotplug lifecycle.  QEMU should never be
>> left with dangling pointers.
>>
> Not sure about the block layer. But I think those thread are already
> out of big lock, so there should be a MemoryListener to catch the
> RAM-unplug event, and if needed, bdrv_flush.


IMO we should use the same mechanism as proposed for other devices:
address_space_map() should grab a reference on the dimm device, and
address_space_unmap() can release it.  This way device destruction will
be deferred as soon as all devices complete I/O.

We will have to be careful with network receive buffers though, since
they can be held indefinitely.

-- 
error compiling committee.c: too many arguments to function