| Message ID | 1364804434-7980-2-git-send-email-qemulist@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 01, 2013 at 04:20:30PM +0800, Liu Ping Fan wrote: > From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> > > This pair of interface are optinal, except for those device which is > used outside the biglock's protection for hot unplug. Not sure if this comment is true. Memory unplug safety is not about the big lock, it's about whether a reference to memory is held *across* a hot unplug operation. So even code that is under the big lock can use a guest RAM buffer across the event loop, and therefore be exposed to a RAM unplug! Therefore inc/dec must be used if guest RAM is held across event loop handler calls. If the guest RAM access happens completely inside a handler function, then it is not affected by hot plug and doesn't need to do inc/dec.
On Thu, Apr 11, 2013 at 5:49 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Mon, Apr 01, 2013 at 04:20:30PM +0800, Liu Ping Fan wrote: >> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com> >> >> This pair of interface are optinal, except for those device which is >> used outside the biglock's protection for hot unplug. > > Not sure if this comment is true. Memory unplug safety is not about the > big lock, it's about whether a reference to memory is held *across* a > hot unplug operation. > What I exactly mean is DeviceX unplug is under biglock, so if operations on DeviceX are all within the biglock, they are safe. But that is not the trueth with RAM. So using ref/unref to manage the reference to memory > So even code that is under the big lock can use a guest RAM buffer > across the event loop, and therefore be exposed to a RAM unplug! > > Therefore inc/dec must be used if guest RAM is held across event loop The ref/unref interface wrapper the inc/dec, exposes them from MemoryRegionOps. > handler calls. If the guest RAM access happens completely inside a > handler function, then it is not affected by hot plug and doesn't need > to do inc/dec. Yes, that is truth. And that is why only calling ref/unref in HostMemListener region_add, but not in mem_add().
diff --git a/include/exec/memory.h b/include/exec/memory.h index 2322732..4289e62 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -55,6 +55,12 @@ struct MemoryRegionIORange { * Memory region callbacks */ struct MemoryRegionOps { + + /* ref/unref pair is optional; ref. + * inc refcnt of object who store MemoryRegion + */ + void (*ref)(void); + void (*unref)(void); /* Read from the memory region. @addr is relative to @mr; @size is * in bytes. */ uint64_t (*read)(void *opaque, @@ -228,6 +234,10 @@ struct MemoryListener { QTAILQ_ENTRY(MemoryListener) link; }; +/**/ +bool memory_region_ref(MemoryRegion *mr); +bool memory_region_unref(MemoryRegion *mr); + /** * memory_region_init: Initialize a memory region * diff --git a/memory.c b/memory.c index 75ca281..c29998d 100644 --- a/memory.c +++ b/memory.c @@ -786,6 +786,24 @@ static bool memory_region_wrong_endianness(MemoryRegion *mr) #endif } +bool memory_region_ref(MemoryRegion *mr) +{ + if (mr->ops && mr->ops->ref) { + mr->ops->ref(); + return true; + } + return false; +} + +bool memory_region_unref(MemoryRegion *mr) +{ + if (mr->ops && mr->ops->unref) { + mr->ops->unref(); + return true; + } + return false; +} + void memory_region_init(MemoryRegion *mr, const char *name, uint64_t size)